window.close does not follow the spec on GeckoView
Categories
(GeckoView :: General, defect, P2)
Tracking
(Not tracked)
People
(Reporter: agi, Unassigned)
References
Details
I don't know how important this is in real life, but window.close does not follow the spec as per this WPT close-method.window.js. The problem is that when we call window.close() we don't set the closed property immediately, even though the spec requires it.
So e.g. a test like this fails in GV
window.close()
assert_equals(window.closed, true);
The reason why the test is currently passing in GV is because they all do something of the form
let w = window.open(...);
// at this point GV delegates haven't registered yet,
// so this close method will not execute any GV code
// and Gecko will handle it instead
w.close();
assert_equals(window.closed, true);
so the window is closed by Gecko but GV or the app are never notified about this. I tested this manually and it seems like Gecko thinks the window is closed but still allows it to navigate and being used anyway (but there could be bugs).
After moving to Actor the .close() method will wake up the actor and make the test fail because now GV will handle the close request.
|
Reporter | |
Comment 1•4 years ago
|
||
One way we could fix this would be to always set the closed property even though the embedder hasn't notified yet (which would mean we're assuming that the embedder correctly closes the window eventually) this would be in line to what Desktop does, which sets the property even though the window isn't closed yet, as that happens on the main process.
|
|
Reporter | |
Comment 2•4 years ago
|
||
Removing this line could be enough to fix this: https://searchfox.org/mozilla-central/rev/3434a9df60373a997263107e6f124fb164ddebf2/mobile/android/chrome/geckoview/GeckoViewContentChild.js#429 (assuming we do want to always mark the window as closed regardless of the embedder's behavior).
|
||
Updated•4 years ago
|
|
|
Reporter | |
Comment 3•4 years ago
|
||
snorp says we should change the name of the API from onCloseRequest to onClose(d) since it's not gonna be optional to close the window anymore.
|
||
Comment 4•2 years ago
|
||
Agi, is this bug still an issue? Makoto says snorp fixed ContentDelegate.onCloseRequest() and window.close WPT in bug 1662528.
|
|
Reporter | |
Comment 5•2 years ago
|
||
Yes it looks like it is. And this was fixed two weeks after I opened this bug and I even reviewed the patch and yet didn't think of coming back here! oh well. Thank you Chris and Makoto.
Description
•