| 06:53 | <annevk> | Shannon Booth sideshowbarker want to review https://github.com/whatwg/dom/pull/1477? |
| 07:29 | <annevk> | I pushed some nits, but I think this latest iteration needs some more work. |
| 07:50 | <Shannon Booth> | left a comment about one extra deviation we have in place, though I think it is separate |
| 07:56 | <Noam Rosenthal> | Oops looking at it again it was a bit sloppy in terms of style. sorry about that. Fixing now. |
| 08:31 | <Noam Rosenthal> | Should be ready for another look. I fixed all the style regressions. |
| 09:40 | <Noam Rosenthal> | Seems like WATTSI is giving a 503 (https://github.com/whatwg/html/pull/12624), anything to do when that happens? |
| 10:13 | <Noam Rosenthal> | ok seems to be fixed the 3rd time |
| 11:39 | <annevk> | Shannon Booth: hello again, I think https://github.com/whatwg/dom/pull/1479 should fix that NodeIterator issue. If you know of a less involved fix I'm all ears. |
| 12:06 | <Noam Rosenthal> | ha not sure why specfmt didn't catch any of these wrapping thing. I couldn't see them myself with the naked eye |
| 12:12 | <annevk> | It's really easy to find them when you look through the complete diff on GitHub and don't see the text lining up. |
| 12:14 | <annevk> | I guess I shouldn't say easy, but it's definitely possible. |
| 12:15 | <Noam Rosenthal> | yea I think I forgot to click "Refresh" in the diff view so I was seeing a previous version |
| 12:16 | <Noam Rosenthal> | (also maybe over-relying on specfmt which was so far rather dependable) |
| 12:23 | <annevk> | But then you should notice that as you read through it surely as it doesn't contain all changes? Your turnaround is quick, but I feel like I spend a lot of time on nits with your PRs. |
| 12:24 | <Noam Rosenthal> | That's fair feedback, I always try to improve. |
| 13:33 | <Noam Rosenthal> | Anyway, for this PR I've now triple-checked everything for nits and I think we're good |
| 14:05 | <Lea Verou> | Another realization: Authors generally want to use the native behavior when it exists. Fundamentally, that is what "tramples all over our design flexibility", not polyfills. Even without a polyfill, authors will use feature detection and use the native functionality if it exists, and fall back to their own code only if it doesn't. This can be done in a userland wrapper, a build tool, or even add hoc per usage. Polyfills are simply abstractions to make this pattern more ergonomic for consumers, whereas a userland wrapper introduces one more level of indirection. IMO, by not working on primitives that make good polyfills possible we do not preserve our design flexibility, we just get a buggier Web… |
| 15:08 | <annevk> | Noam Rosenthal: for https://github.com/whatwg/dom/pull/1478#pullrequestreview-4617350032 what happens if you clone a document and then document.open() it? This will give different results depending on how implementations track this state. |
| 15:09 | <annevk> | Noam Rosenthal: you probably don't even have to clone it depending on when a new parser is allocated. |
| 15:10 | <annevk> | Right so I think you change the document open steps so they always set this to true. Which means that an XHR document with document.open() called on it will now do something different. At least specification-wise. Right? |
| 15:14 | <Noam Rosenthal> | Right! That edge scenario is untested and the PR changes the behavior. Though I'm not sure what the correct behavior is. I think document.open() always allowing (or not allowing) DSD regardless of where the document came from makes more sense(TM) when you think of what this flag comes to protect against. |
| 15:15 | <annevk> | I think I agree it's fine, but we should be intentional about it in terms of tests and eventual commit message. |
| 15:16 | <Noam Rosenthal> | Absolutely. I'll add a test for this scenario and update the commit message. I didn't know about the XHR subtlety (probably because it's not tested and I'm not sure if blink does the spec'ed thing there...) |
| 15:28 | <Noam Rosenthal> | annevk: Ah actually blink does keep this flag for non-fragment parsing and it affects document.write() in those scenarios like XHR (also document.implementation.createHTMLDocument). So I think we should keep this flag in DOM but stop override it when it's the fragment parser rather than rely on the state of the inert document. I'll revise the HTML PR |
| 15:28 | <Noam Rosenthal> | We can later think of changing the behavior but the goal here was to better spec current behavior so lets start with that |
| 16:48 | <Noam Rosenthal> | OK I updated the PR and commit description, as well as a link to a test that verifies what happens with document.open() after XHR |