Don't set the "fired unload" flag for document.open() #1018
Conversation
|
Not setting the flag like that is a bit worrisome. In particular, it's assuming that there will be no script running between that point and the poing when we'd reset the flag to false, right? I don't really have the time to sort through all the little twisty side-effects and determine whether this condition holds or not. I think it would be a lot safer to set the flag normally, then unset it in the steps of |
|
I think you are right, in particular due to the unloading of child browsing contexts. Thanks. |
domenic
force-pushed
the
fix-unload-flag
branch
from
7171371 to
addd116
Compare
|
Fixed to just reset the flag, as suggested in the original bug. Probably any old editor can review this now. |
|
I'm not old... The change looks okay, although I don't fully grasp all the implications of this yet admittedly. |
domenic
force-pushed
the
fix-unload-flag
branch
from
addd116 to
1f6fa09
Compare
Fixes https://www.w3.org/Bugs/Public/show_bug.cgi?id=28772.
This needs pretty careful review to see if it's actually capturing the intention of that bug and of the original algorithm. @bzbarsky, would appreciate your help here, especially since I took a different approach (of not setting the flag for document.open() at all) than you suggested (of re-setting the flag to false at some point).