| 08:26 | <yoav> | annevk: Hey! Trying something out to see if we can reasonably show long-standing PR diffs in a reasonable way. Is there a way to "auto collapse" the commit-snapshot warning? |
| 08:41 | <annevk> | yoav: no |
| 08:42 | <yoav> | It is OK if I add something like that? |
| 08:42 | <annevk> | I don't think so, that design is quite intentional |
| 08:44 | <yoav> | I understand why that design is the way it is, but want to show the diff in a concise way, so want iframes showing bits and pieces of it |
| 08:45 | <yoav> | having to click collapse on every iframe would defeat the purpose |
| 08:46 | <annevk> | If we provided a workaround, how would that not open us to abuse? |
| 08:46 | <yoav> | I guess I could drive the same with JS |
| 08:46 | <yoav> | nm |
| 09:49 | <yoav> | annevk: would you be comfortable with a postMessage based mechanism that tells the preview to scroll to a certain anchor? |
| 09:49 | <yoav> | changing the src from the outside seems to trigger a new navigation |
| 09:50 | <yoav> | (at least in my initial testing. Maybe there's a way around that) |
| 09:51 | <yoav> | that's wasteful + after a certain point I'm getting 403s, presumably for requesting the same preview too many times |
| 09:52 | <annevk> | yoav: it'd be helpful to have an issue somewhere with a problem description or some such |
| 09:53 | <yoav> | sure, but wondered what your initial thoughts would be before I go to fully document it |
| 09:55 | <annevk> | yoav: somewhere between maybe and wondering if we need X-Frame-Options: deny 😊 |
| 09:56 | <yoav> | my immediate goal is to make PRs like https://github.com/whatwg/fetch/pull/773 and https://github.com/whatwg/html/pull/3774 more comprehensible to casual reviewers |
| 09:57 | <yoav> | So I won't need to create a separate monkey patching document |
| 09:58 | <yoav> | so I'm trying to create a document which frames the PR preview, talks about what each change in it does and when the user clicks on a link, I want the PR preview to scroll to the relevant section |
| 09:59 | <yoav> | I can open an issue, but not 100% sure where that issue should go to. Fetch? HTML? PR preview? |
| 09:59 | <annevk> | yoav: https://github.com/whatwg/whatwg.org I suspect |
| 10:00 | <yoav> | ok |
| 10:00 | <annevk> | yoav: it's not possible to explain the set of changes and link to them? |
| 10:01 | <yoav> | It's possible, but will force the person doing the casual reviewing to hop back and forth between the explanation and the spec language |
| 10:01 | <yoav> | having both side by side seems better |
| 10:16 | <yoav> | annevk: https://github.com/whatwg/whatwg.org/issues/241 |
| 13:58 | <annevk> | nox: so for <script> Chrome and Firefox presumably only disagree on details? |
| 13:58 | <annevk> | nox: as both have the defer mechanism |
| 13:58 | <nox> | annevk: Seems like a huge detail to me. |
| 13:58 | <annevk> | nox: I guess I can run the tests |
| 13:59 | <nox> | annevk: If you insert 2 scripts and modify the second from the first, |
| 13:59 | <nox> | the second will be executed at the time it is modified in Chrome, |
| 13:59 | <nox> | whereas in Firefox it will not execute immediately. |
| 13:59 | <nox> | annevk: I described what FIrefox does in a comment, let me find it. |
| 13:59 | <nox> | annevk: https://github.com/whatwg/html/pull/4354#issuecomment-463566900 |
| 17:08 | <annevk> | nox: okay, is either solution better in some way? |
| 17:08 | <annevk> | nox: the Firefox solution seems a little cleaner in that original ordering is preserved |
| 17:09 | <nox> | annevk: I personally prefer the Chrome behaviour because it’s more in line with what I expected. |
| 17:10 | <annevk> | nox: are there any tradeoffs you see? |
| 17:11 | <annevk> | nox: if it's just edge cases I don't care strongly I suppose |
| 17:11 | <annevk> | nox: maybe I should run your stuff in Safari |
| 17:11 | <nox> | annevk: It’s only ever exhibited in rare edge cases yeah. |
| 17:12 | <nox> | annevk: Safari doesn’t defer anything AFAIK, especially with fragments given it inserts children separately as if you had called insertBefore repeatedly yourself. |
| 17:13 | <annevk> | Yeah, Safari seems out there with these tests |
| 17:14 | <annevk> | nox: a problem I have with reviewing is that the alternatives are not written down in one place, so it's hard to judge things |
| 17:15 | <nox> | annevk: I’m also not sure what happens if you insert two scripts and the first removes the second in Firefox |
| 17:15 | <annevk> | nox: tkent wrote some changes down, but they don't really concern what Chrome does with respect to script insertion |
| 17:15 | <annevk> | nox: I'd epxect the second to not execute based on what I know |
| 17:15 | <nox> | Specifically, is the already-started flag reset? |
| 17:16 | <nox> | Ah never mind yeah it will be cleared out ultimately given it’s not removed from the queue of deferred things. |
| 17:29 | <annevk> | nox: so is "text content changed" misleadingly named? What you set node.data only? |
| 17:30 | <nox> | I don't understand the second question and it's Friday evening so I don't really want to think about the first one hah. |
| 17:31 | <annevk> | It's not misleadingly named, it's also called when you set node.data |
| 17:32 | <devsnek> | interesting api from node: https://github.com/nodejs/node/pull/26686 |
| 17:32 | <annevk> | Could still be a single "children changed" callback though |
| 17:40 | <nox> | annevk: AFAIK inserting non-Text nodes in a style doesn't reset the stylesheet. |
| 17:42 | <annevk> | Yeah, I think Domenic tested this too perhaps; I wonder what browsers do in that case, have some kind of filter |
| 17:42 | <annevk> | A thing they might do is simply compare input to the parser to previous input and return early, though it seems somewhat wasteful to keep all of that around |
| 17:43 | annevk | goes to look for tests |
| 17:49 | <annevk> | Hmm, so when the concept was added to DOM no tests were added because only textarea was considered at the time |
| 17:49 | <annevk> | So whether node.normalize() affects <style> is unclear |
| 17:52 | <annevk> | And per https://github.com/whatwg/html/pull/3024 we did not add tests when this was also adopted by <style> |
| 17:52 | <devsnek> | what is importing html supposed to do |
| 17:53 | <annevk> | Okay, at least that makes it clear what needs to be done |
| 17:53 | <devsnek> | from the explainer it looks like content is an element and default is a vdom? |
| 17:53 | <annevk> | devsnek: HTML modules? |
| 17:53 | <devsnek> | yeah |
| 17:53 | <annevk> | devsnek: it's a fancy xhr.responseXML |
| 17:54 | <devsnek> | aha |
| 17:54 | <devsnek> | thank you |
| 17:54 | <annevk> | I'm surprised that worked 😊 |
| 17:55 | <devsnek> | annevk: i was just trying to figure out if html modules would be affected by the (non)existence of top level await |
| 18:05 | <annevk> | nox: I created the most basic test |
| 18:05 | <annevk> | nox: inserting a comment node into a <style> |
| 18:06 | <annevk> | nox: sheet changed |
| 18:08 | <nox> | Ah, then I don't remember why I kept the distinction. |
| 18:09 | <annevk> | https://github.com/web-platform-tests/wpt/pull/15871 |
| 18:09 | <nox> | annevk: Oh, the opposite. |
| 18:09 | <annevk> | I might add some more tests next week, but I'm convinced now that we should only have "children changed" and not both of these concepts |
| 18:12 | <nox> | I'm pretty sure I kept it for a reason but I could be wrong. |
| 18:14 | <nox> | annevk: Ah now I remember. |
| 18:14 | <nox> | annevk: The text content changed stuff is also called when changing Text nodes' contents and whatnot, |
| 18:14 | <nox> | annevk: changing the contents of a Text node in a script element doesn't do anything to the script element. |
| 18:15 | <nox> | script doesn't define any "child text content change steps", but it does say to prepare it whenever "The script element is connected and a node or document fragment is inserted into the script element, after any script elements inserted at that time." |
| 18:18 | <annevk> | nox: interesting, I want to see if we can change that if true or perhaps we can make that a parameter |
| 18:18 | <nox> | annevk: Meh, I would rather avoid ad-hoc use-once boolean parameters. |
| 18:19 | <annevk> | nox: I’d much rather avoid two subtly different callbacks with one only used once |
| 18:19 | <nox> | I wouldn't call the difference subtle at all. |
| 18:20 | <nox> | Not sure what you mean by "used only once" though. |
| 18:21 | <annevk> | nox: for scripts |
| 18:21 | <annevk> | nox: it’d be good to see that test though |
| 18:23 | <annevk> | And please don’t cite the current spec in support of anything as impls are all over 😊 |
| 18:34 | <nox> | I'm pretty sure I verified the thing about changing a text child, but yeah didn't write a test. |
| 18:34 | <TabAtkins> | Where's the statement in HTML that UAs are allowed to fail due to resource constraints whenever they want? |
| 18:38 | <TabAtkins> | Eh, nm, I just cited it as a general principle instead. |
| 19:17 | <Domenic> | TabAtkins: https://html.spec.whatwg.org/#killing-scripts at least if you're talking about running scripts |
| 19:21 | <TabAtkins> | That is indeed what I was looking for, but I didn't realize it was so limited to just scripts. |
| 19:23 | <Domenic> | Yeah I couldn't find anything more general. It might be hiding, but I Ctrl+F'ed "constraints" and "resources" and didn't see anything early in the spec |
| 19:25 | <TabAtkins> | Asking here too, as w3c#testing is quiet right now: how do you test something that's just "don't let me do X enough times to make the browser crash"? |
| 19:26 | <TabAtkins> | (Context is a Billion-Laughs attack via CSS variables; the spec now mandates that browsers must protect themselves from it, but doesn't specify a specific maximum length.) |
| 19:28 | <Domenic> | TabAtkins: I think you write a test with no asserts and if it crashes the harness/infra will call that a failure. If it doesn't crash then a no-assert test is a pass. |
| 20:14 | <annevk> | TabAtkins: Infra might say that? About resource limits |
| 21:30 | <TabAtkins> | Domenic: Thanks, https://github.com/web-platform-tests/wpt/pull/15875 is committed now, and does indeed crash/fail in Chrome. ^_^ |
| 21:31 | <Domenic> | Amazing |
| 21:31 | <astearns> | that's a good test :) |
| 22:43 | <TabAtkins> | Domenic: FYI, apparently a vacuous test without an assert is *not* a pass; it just times out and counts as a failure. |
| 22:45 | <Domenic> | TabAtkins: I said "without asserts", not "without tests" :) |
| 22:45 | <Domenic> | (I certainly could have been clearer) |
| 22:45 | <TabAtkins> | Bah! |
| 22:45 | <TabAtkins> | ^_^ |
| 22:50 | <Mek> | well, you don't need explicit tests or asserts, you just need to call done() if you have neither (https://web-platform-tests.org/writing-tests/testharness-api.html#single-page-tests) |