| 06:20 | <annevk> | Anyone know the details about the unload event fire in Gecko? Per https://software.hixie.ch/utilities/js/live-dom-viewer/saved/14453 it looks like it fires after removal somehow? |
| 07:53 | <nektro> | ended up answering my unicode question and finished my Zig IDNA implementation https://github.com/nektro/zig-unicode-idna :) |
| 07:53 | <nektro> | now to get back to url hehe |
| 08:22 | <sideshowbarker> | So everybody else fires it after? |
| 08:23 | <sideshowbarker> | per spec, is the removal sync? And is the firing of the event async? |
| 08:24 | <sideshowbarker> | I’m trying to do some code inspection, looking at the code locally — but… it’s pretty hairy |
| 08:24 | <sideshowbarker> | I kind of feel like nobody might know what the code is doing — except whoever wrote it or last had to touch it |
| 08:25 | <sideshowbarker> | and probably they don’t even remember by now |
| 08:25 | <sideshowbarker> | but then I guess there’s a high change “they” = smaug anyway |
| 08:29 | <sideshowbarker> | … anyway, in other news, since I’m going to be the W3C staff contact for the Web Extensions WG, I figured I should actually try to write a cross-browser extension — in order to fully participate in the pain of trying to do that — and I managed to come up with an actually-pretty-useful extension: |
| 08:29 | <sideshowbarker> | https://github.com/sideshowbarker/social-media-cleanser |
| 08:30 | <sideshowbarker> | I encourage any and all try to it, and specifically lemme know if you run into any install problems. And otherwise of course, does it actually work for you. |
| 08:38 | <annevk> | In https://github.com/whatwg/html/issues/4611#issuecomment-1960779830 Dominic Farolino claimed all browsers are aligned, but my trivial example from above shows that's not the case. |
| 08:42 | <sideshowbarker> | seems like it’s actually async in gecko at least |
| 08:50 | <annevk> | I don't think so. At least per https://software.hixie.ch/utilities/js/live-dom-viewer/saved/14454 it seems like they might just clear frameElement earlier, but if you cache it in some other way you still get to make changes around the same time. But perhaps maybe it's slightly delayed? |
| 09:11 | <annevk> | I'm looking at this since rniwa is saying selectedcontent is unsafe as specified because of the unload event. I'm curious how jarhar dealt with this as I don't really see anything obvious in the Chromium code. I did notice Chromium doesn't appear to clear all non-primary selectedcontent elements when testing. |
| 11:09 | <smaug> | We were looking into something similar very recently on Gecko side and selectedcontent. I thought it was safe after all, but just possibly a bit weird |
| 11:10 | <smaug> | jjaschke might recall |
| 11:10 | <smaug> | Something about remove all the child nodes |
| 11:13 | <smaug> | And yes, I couldn't immediately find the code in Chromium doing what the spec says, but didn't spend much time on that |
| 11:14 | <jjaschke> | Selectedcontent calls replace all with when cloning stuff into it or removing its contents |
| 11:31 | <smaug> | (and similar-ish weirdness may happen for example when using range.deleteContents(), so nothing new there) |
| 11:33 | <Noam Rosenthal> | so the unsafe thing here is that an <iframe> inside a <selectedcontent> can trigger a pagehide that would observe some mid-algorithm state, or some such? |
| 11:34 | <smaug> | yeah, pagehide or unload. That was the worry I had. Not sure if annevk is talking about something else |
| 11:34 | <Noam Rosenthal> | (note that while unload is deprecated and has weird cross-browser issues, pagehide is relatively interoperable I think and potentially has the same issue so it's easier to reason about) |
| 11:35 | <smaug> | sure |
| 13:20 | <annevk> | Noam Rosenthal: is pagehide synchronous as well? If so, yes. I think we need post-removing steps, maybe? |
| 13:21 | <Noam Rosenthal> | Noam Rosenthal: is |
| 13:23 | <annevk> | I'll just make the issue about pagehide then. |
| 13:25 | <annevk> | https://github.com/whatwg/html/issues/12096 |
| 13:30 | <annevk> | smaug: yeah rniwa was a bit stricter than you I suppose and just said no. 😊 |
| 13:30 | <Noam Rosenthal> | @annevk:matrix.org: not sure if post removal steps would help. It's a really tricky one. We explored it when dealing with moveBefore though it ended up not being an issue there because moveBefore surpresses iframe removal. Will comment on the issue |
| 13:31 | <Noam Rosenthal> | The safest thing would be to disallow subframes in selectedcontent but I don't know if it breaks use cases or not |
| 13:33 | <annevk> | If we remove the nesting by postponing the removal I'm not sure what issue would remain? It does seem possible someone can think of use cases for selecting between nested documents, even if a bit absurd. |
| 13:34 | <Noam Rosenthal> | Oh postponing the removal specifically in selectedcontent? That would also work. |
| 13:34 | <annevk> | Right, specifically in the case where removal would otherwise end up nesting. |
| 15:13 | <Dominic Farolino> | annevk: I'm not sure what "remove the nesting by postponing the removal" means? Do you mean run the pagehide first, and postpone the actual DOM removal until after that event has fired? |
| 15:21 | <annevk> | Dominic Farolino: we try to do removal during the removal of the selectedcontent itself. That's nesting. If we postpone that removal we can avoid the nesting. I'd be interested to learn more about smaug's deleteContents() case as I guess that can do something similar due to it performing multiple tree operations. We might actually have to define some of the mutation event defenses because of pagehide. |
| 15:48 | <Dominic Farolino> | annevk: Didn't know what you meant by nesting, gotcha. Is "nesting" removing steps any worse than the general problem with removal steps, namely that iframes fire pagehide syncly on removal when the spec says it doesn't? If a selected content is cleared and an iframe's pagehide fires, can it observe state that's any less consistent than if an ordinary iframe's parent div gets removed? There, the child iframe's removing steps are still called, and observe inconsistent state wrt its siblings' removing steps side-effects. |
| 15:49 | <Dominic Farolino> | Of course, it's unfortunate that selected content APIs bake this in natively which is a shame. I'm annoyed I missed this in review. |
| 15:57 | <annevk> | I don't know how this works in other implementations and the specification is definitely wrong on this, but WebKit has a very specific path for "disconnectSubframesIfNeeded" that's called during node removal at a particular point in time. With this nesting it can also be called at other points in time, which is what makes this risky. I'm also not an expert on this. rniwa is, but he has limited bandwidth. |
| 16:01 | <annevk> | And come to think of it, maybe that is also why deleteContents() is not problematic, because we complete each removal before starting the next one. |
| 16:19 | <Dominic Farolino> | I suppose... Still, when deleteContents() runs the removing steps for a doomed iframe, its pagehide handler can easily remove another iframe elsewhere, thus nesting removing steps. I wonder if that imposes the same risk in WebKit as the native selected content nesting. (Neither is acceptable, FWIW... all of it is bad) |
| 16:26 | <annevk> | No, pagehide running remove during remove is expected. What is unexpected is an arbitrary element running remove (during remove). |
| 16:26 | <annevk> | Because the latter means script can now run at a new point in time that is not guarded against whatsoever. |
| 19:33 | <Dominic Farolino> | annevk: We discussed this today and I think we have a path for removing the selected content's removing steps altogether |
| 20:57 | <smaug> | annevk: about deleteContents. I think that algorithm is actually broken. The last step may set the start and end boundary points pointing to an invalid place in DOM if remove step above modifies DOM and for example removes all the children from newNode before newOffset. |