Fix the #savedHasOwnCanvas handling in the StampAnnotation class
#19619
Conversation
This may not be possible to trigger in practice, however it seems that if `StampAnnotation.prototype.mustBeViewedWhenEditing` is called back-to-back with `isEditing === true` set then the second invocation could overwrite the `#savedHasOwnCanvas`-field and thus lose its initial state.
|
@calixteman It's very possible that I've completely misunderstood this code, in which case sorry for wasting your time, but to me it appears that there's a potential problem with it. |
There was a problem hiding this comment.
I'd have said that only this.#savedHasOwnCanvas = this.data.hasOwnCanvas; should be removed.
But I think your implementation is safer, at least in case the value data.hasOwnCanvas is changed for whatever reason.
Since it's pretty hard to test in general, I think the safest solution is the best.
LGTM. Thank you.
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/93549dd9dd0d911/output.txt |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/2b60b109bf48673/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/2b60b109bf48673/output.txt Total script time: 30.72 mins
|
|
/botio-linux integrationtest |
From: Bot.io (Linux m4)ReceivedCommand cmd_integrationtest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/3c0068fb55dabf4/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/3c0068fb55dabf4/output.txt Total script time: 11.85 mins
|
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/93549dd9dd0d911/output.txt Total script time: 57.99 mins
Image differences available at: http://54.193.163.58:8877/93549dd9dd0d911/reftest-analyzer.html#web=eq.log |
|
The Windows ref-tests "failures" are because makeref didn't run (because of a broken bot) after the ICC PR landed the other day. /botio-windows makeref |
From: Bot.io (Windows)ReceivedCommand cmd_makeref from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/89fb7cd4596fde7/output.txt |
From: Bot.io (Windows)SuccessFull output at http://54.193.163.58:8877/89fb7cd4596fde7/output.txt Total script time: 29.63 mins
|
This may not be possible to trigger in practice, however it seems that if
StampAnnotation.prototype.mustBeViewedWhenEditingis called back-to-back withisEditing === trueset then the second invocation could overwrite the#savedHasOwnCanvas-field and thus lose its initial state.