| 11:02 | <annevk> | nox: for any future PRs to DOM you should now be able to create branches directly on the main repository (please don't push to master) |
| 11:03 | <annevk> | nox: the advantage of that is that the person committing (mostly me I guess) can rebase before landing, resulting in automatic closure of the PR and some automatic cross-referencing of the commit to the PR |
| 11:05 | <nox> | annevk: Ok cool. |
| 11:07 | <ritsyy> | annevk: https://github.com/whatwg/html/pull/748 reviews for this one?, i am unclear about some of the sections |
| 11:10 | <annevk> | ritsyy: looking |
| 11:14 | <annevk> | ritsyy: reviewed |
| 11:15 | <ritsyy> | ritsyy: yes, renaming it, thanks! |
| 11:15 | <annevk> | ritsyy: thanks for the ping, if you pushed that commit a few days ago, remember to say something in the issue, otherwise reviewers won't get a notification |
| 11:16 | <ritsyy> | annevk: no no i just did the commit, i thought to ping you here, will comment there too after this commit |
| 11:18 | <annevk> | ah okay |
| 11:19 | <ralt> | annevk: might be worth to switch to gitlab, it has many features that you seem to want (rebase in PRs, protected branches and permissions let some people push to non-protected branches only, etc) |
| 11:19 | <ralt> | or at least, look at gitlab :) |
| 11:22 | <annevk> | ralt: wow cool, I want that, but with the community from GitHub |
| 11:23 | <annevk> | It was already hard to imagine we're the only ones with these issues |
| 12:00 | <nox> | annevk: https://github.com/tabatkins/bikeshed/commit/152f137ad3946c665c8a78648ac0d803369642b5 |
| 12:00 | <nox> | That's the bug I think. |
| 12:05 | <nox> | annevk: https://github.com/tabatkins/bikeshed/pull/611 |
| 13:27 | <nox> | annevk: OOOOOH, I think I got a workaround to unblock DOM. |
| 13:28 | <annevk> | Call TabAtkins? |
| 13:29 | <nox> | annevk: Nah. |
| 13:29 | <nox> | And I failed. :( |
| 13:29 | <nox> | annevk: I hoped = would do the trick given it's just regexps. :P |
| 13:31 | <nox> | annevk: We can also do 'src= "insanity-wolf"' for now. |
| 13:31 | <annevk> | Nah, let's wait |
| 13:32 | <annevk> | TabAtkins discouraged me to work around flaws |
| 13:33 | <nox> | K. |
| 15:20 | <Domenic> | We should set up CI for the bikeshed specs so that (a) we don't have to check in generated code; (b) we can pin to a specific Bikeshed version. |
| 15:20 | <Domenic> | I envision a metadata file in the repo (maybe .travis.yml, maybe something else) that contains the Bikeshed commit hash that the spec is meant to be built with |
| 15:20 | <Domenic> | And you can update that every once in a while |
| 15:32 | <annevk> | I would not mind such a setup |
| 15:39 | <gsnedders> | I guess the important question is whether we have any desire to review BS output. Probably not really, because it's just too noisy. |
| 15:45 | <annevk> | gsnedders: it's somewhat useful to do, especially since I often type <span> instead of <a>, but it's very noisy indeed |
| 16:02 | <Ms2ger> | TIL about the word "excrescence" |
| 16:16 | <annevk> | 1 of 1 matches |
| 16:23 | <Ms2ger> | foolip++ |
| 16:24 | <philipj> | Ms2ger: I tried quite hard to find something to complain about, but the truth is you write tests almost exactly like I would aspire to :) |
| 16:26 | <philipj> | Ms2ger: if you're in a good mood, the top two in https://github.com/w3c/web-platform-tests/pulls/foolip need review |
| 16:26 | philipj | does actually know what mood has to do with it... |
| 16:26 | <philipj> | doesn't! |
| 16:29 | <Ms2ger> | philipj, r+ and r+ with rebase and nit |
| 16:35 | <Domenic> | While we're talking tests... is https://github.com/yaycmyk/jsdom/blob/label-input-activation/test/web-platform-tests/to-upstream/html/semantics/forms/the-label-element/proxy-click-to-associated-element.html#L24-L29 the correct pattern for testing events, or should we be using EventWatcher maybe, or...? |
| 16:35 | <Domenic> | EventWatcher seems pretty heavyweight for single events I guess |
| 16:36 | annevk | wonders if nox has studied Shadow DOM yet |
| 16:36 | <nox> | Not yet. |
| 16:36 | <nox> | But given I implemented <template>, I probably will at some point. |
| 16:36 | <Ms2ger> | Domenic, assert_true(true); is dumb |
| 16:37 | <Domenic> | Ms2ger: just do t.done()? |
| 16:37 | <Ms2ger> | Domenic, input.addEventListener("click", t.step_func_done()) is probably what I'd use |
| 16:37 | <Domenic> | Ms2ger: hmm, that's not documented in http://testthewebforward.org/docs/testharness-library.html#promise-tests |
| 16:38 | <Ms2ger> | You're correct |
| 16:39 | <Domenic> | I'll see if I can pull off a PR from the web interface |
| 16:39 | <annevk> | Domenic: ETA on structured clone? |
| 16:39 | <philipj> | Ms2ger: thanks! |
| 16:39 | <Domenic> | annevk: within the hour |
| 16:39 | <Domenic> | Ms2ger: oh lol I see why they did assert_true. The docs told them to. |
| 16:39 | <Domenic> | Ms2ger: "The above example can be rewritten as:" |
| 16:40 | <annevk> | Domenic: I want to make the change I suggested in the PR, but other than that I think it's good, but who knows, you might find something again 😃 |
| 16:40 | <Ms2ger> | Huh |
| 16:40 | <annevk> | Domenic: in the PR thread* |
| 16:41 | <Domenic> | I will change it to assert_equals(1+1, 2) |
| 16:48 | <Ms2ger> | Domenic, are you updating the docs? |
| 16:48 | <Domenic> | Ms2ger: https://github.com/w3c/testharness.js/pull/186 |
| 16:51 | <Ms2ger> | Domenic, reviewed |
| 16:51 | <Domenic> | Ms2ger: dang, now I have to go make changes using not the web interface :P. Will fix after reviewing this HTML PR... |
| 16:52 | <Ms2ger> | Domenic, I'll review again tomorrow if nobody beats me to it :) |
| 17:10 | <TabAtkins> | annevk: I'm working on it this morning, don't worry. |
| 17:12 | <TabAtkins> | nox's PR seems hacky but workable, I'll merge in a few minutes. |
| 17:13 | <nox> | TabAtkins: It doesn't pass tests. |
| 17:13 | <nox> | TabAtkins: I'm not sure what's that thing is supposed to match btw, because I don't see =type= in that shorthands file. |
| 17:13 | <TabAtkins> | Yeah, that's why I can't merge from my phone. |
| 17:13 | <TabAtkins> | Haven't updated the docs yet, was waiting for any problems to shake out. 😀 |
| 17:22 | <annevk> | ugh |
| 17:22 | <annevk> | TabAtkins: it basically delayed fixing stuff in DOM by two days, that's not a great way to operate |
| 17:22 | <annevk> | TabAtkins: please fix the way you develop bikeshed |
| 17:26 | <annevk> | Domenic: const a = []; Object.defineProperty(a, "length", { get() { return 5; } }) throws |
| 17:26 | <annevk> | Domenic: I based my code on similar-looking code in ECMAScript |
| 17:27 | <Domenic> | annevk: hmm, I don't think it should, per spec... |
| 17:27 | <Domenic> | annevk: ah I see, length is non-configurable... |
| 17:28 | <Domenic> | annevk: OK, that part looks good |
| 17:32 | <annevk> | Domenic: should we go through the placeholder stuff here? |
| 17:32 | <annevk> | Domenic: or is it clear now? |
| 17:32 | <Domenic> | annevk: it is definitely not clear, but I will try re-reading it with your new explanation in mind and suggesting better text... |
| 17:32 | <annevk> | Domenic: this is basically what the old text had too |
| 17:33 | <annevk> | Domenic: though replacing the placeholder with the transferred object lived in postMessage() |
| 17:33 | <Domenic> | annevk: well, I didn't review the old text :) |
| 17:33 | <annevk> | Domenic: sure, but basically a lot of improvements are being blocked on this now |
| 17:34 | <Domenic> | annevk: could it be rephrased as "Everywhere placeholderResult shows up inside clone, replace it with transferResult"? |
| 17:34 | <annevk> | Domenic: basically if you have an object-graph in your engine, you want to take the placeholder object and replace it directly with the transferred object |
| 17:34 | <Domenic> | annevk: right but e.g. does that mean crawling the graph with GetOwnProperties etc.? |
| 17:35 | <annevk> | Domenic: no, because then if it didn't show up inside clone it wouldn't be replaced |
| 17:35 | <Domenic> | annevk: that is the main part of my question. where else can it show up. |
| 17:35 | <annevk> | Domenic: I'm not sure what GetOwnProperties is or how it relates |
| 17:36 | <annevk> | Domenic: it's returned in [[TransferList]] |
| 17:36 | <annevk> | Domenic: hmm, maybe it cannot show up there, those don't use memory at the moment |
| 17:36 | <Domenic> | annevk: right that's step 5.4. But in 5.3, which we are discussing, it isn't in outputTransferList |
| 17:37 | <annevk> | Domenic: I was thinking that objects ending up in [[TransferList]] could have references, but that's currently not possible |
| 17:37 | <annevk> | Domenic: that might change though if we start passing memory to [[Transfer]] in the future for some new use case |
| 17:38 | <Domenic> | annevk: let's worry about that then? |
| 17:38 | <annevk> | Domenic: sure, I can add a comment under [[Transfer]] similar to what I have under [[Clone]] |
| 17:39 | <Domenic> | What kind of comment? |
| 17:39 | <Domenic> | Oh you mean <!-- comment |
| 17:39 | <annevk> | <!-- if we add memory, we need to change so and so --> |
| 17:39 | <Domenic> | Thanks. |
| 17:39 | <Domenic> | This pushes the major imprecision into the single phrase "inside clone", which we can live with |
| 17:40 | <Domenic> | Ideally we'd specify exactly how you go crawling the object graph and replacing these placeholders. Or figure out what implementations actually do. |
| 17:40 | <TabAtkins> | All right, nox's fix worked, dom builds cleanly now. |
| 17:40 | <annevk> | I see, I thought that was more or less a given since we created placeholder objects inside the StructuredCloneWithTransfer algorithm and that algorithm only ever returns clone/transferList or an exception |
| 17:42 | <annevk> | Domenic: ^ still not entirely clear to me why you think it's unclear now, but I guess I can add "in /clone/" |
| 17:43 | <Domenic> | annevk: I am trying to figure out if there are observable consequences to different ways of doing this replacement (because there are many: different ways of graph traversal; different ways of getting the list of properties; etc.) but I am not sure yet one way or the other. |
| 17:43 | <annevk> | I suspect this actually happens at a much lower level than getting properties in engines |
| 17:44 | <annevk> | They have these placeholder objects allocated with some pointer, and they just make that point someplace else |
| 17:44 | <Domenic> | Right, I guess so. |
| 17:44 | <TabAtkins> | nox: Unfortunately using entities doesn't avoid the shorthands, as all but a few run over the parsed DOM, not raw text, so they see the entity already substituted. Haven't come up with a good way around that yet. :/ |
| 17:45 | <nox> | TabAtkins: Yeah understood that afterwards. :) I saw regexps and just thought bikeshed was crazier than I had thought initially. Sorry for that. :P |
| 17:46 | <annevk> | Domenic: it would only be observable if you could get cause code to run in targetRealm while cloning, which should be impossible |
| 17:46 | <annevk> | Domenic: I agree that it would be nice if we had better language to define this, but I don't think ECMAScript gives low-level access to allocated objects |
| 17:47 | <TabAtkins> | nox: I'd actually thought of the case that DOM hit, but remembered that it wouldn't be hit by normal attributes. I did not consider code examples. ^_^ |
| 17:47 | <annevk> | Domenic: maybe I should ask in #jslang if anyone has ideas |
| 17:47 | <TabAtkins> | Anyway, DOM is being put into the integration suite now, and I'm gonna put a bunch more in too, so it's unlikely I'll break anything without noticing again. |
| 17:51 | <annevk> | TabAtkins: thank you, dom.bs and url.bs are the main pain points |
| 17:51 | <annevk> | TabAtkins: notifications.bs seems to get by, mostly |
| 17:52 | <annevk> | Domenic: asked there, but no high hopes |
| 17:52 | <TabAtkins> | annevk: kk, I'll put url.bs in too. No reason not to, testing is already slow enough that slowing it down by another minute or so is fine. |
| 17:52 | <Domenic> | annevk: just clarifying that it's scoped to clone is enough for now for me |
| 17:52 | <annevk> | TabAtkins: you might want to improve those build times :-P |
| 17:53 | <TabAtkins> | Man don't I know it. Got a few things lined up to work on for that, then gonna dive in and see what else is slowing things down. |
| 17:53 | <TabAtkins> | Got a decent profiler already hooked up, so it's just a matter of putting in the time. |
| 18:13 | <IZh> | Hi. Is this a valid link? <a href=#shared-internal-slot:-[[crossoriginpropertydescriptormap]]>...</a> |
| 18:15 | <annevk> | think so |
| 18:15 | <IZh> | annevk: Shouldn't square brackets be %-encoded? |
| 18:16 | <annevk> | IZh: I guess per https://url.spec.whatwg.org/#syntax-url-fragment they should |
| 18:17 | <annevk> | IZh: that might be hard to fix though |
| 18:17 | <IZh> | annevk: Well, the spec has only one wrong anchor. |
| 18:18 | <annevk> | IZh: it's generated, that's the problem |
| 18:18 | <annevk> | IZh: although I suppose we could just override the id of that header and forget about it |
| 18:19 | <IZh> | annevk: But why not to fix escaping code? I suppose it must already %-encode non-ASCII symbols. Why not just add the square brackets to the set? |
| 18:19 | <TabAtkins> | annevk: I'm not url-encoding anchors in several spots, I'm happy to go through and fix that. |
| 18:20 | <TabAtkins> | (I haven't needed to be precise with that, because Bikeshed only generates IDs that are url-safe without any escaping needed.) |
| 18:22 | <Domenic> | This is Wattsi though :) |
| 18:24 | <IZh> | It seems that https is not supported on IPv6 address of whatwg.org. |
| 18:27 | <annevk> | IZh: as I said, that might be tricky |
| 18:27 | <annevk> | IZh: file an issue against whatwg/wattsi |
| 18:27 | <annevk> | IZh: or a PR 😃 |
| 18:30 | <annevk> | Domenic: if you can take another look now maybe we can land it today |
| 18:31 | <IZh> | annevk: Filed #22. |
| 18:33 | <annevk> | IZh: thank you, is this breaking anything for you or just something you noticed? |
| 18:34 | <IZh> | annevk: Just something another validator is barking on. :-) |
| 18:34 | <IZh> | annevk: I'm periodically validating the spec. :-) |
| 18:35 | <IZh> | Because the spec has to conform to itself. |
| 18:37 | <annevk> | Domenic: would "across event loops, i.e., ..." work? |
| 18:37 | <annevk> | Domenic: I mainly don't like the ":" |
| 18:38 | <Domenic> | Yeah it would. I like the colon slightly more but both should be ok |
| 18:39 | <Domenic> | Heading upstairs from lunch now, we can indeed probably land this today |
| 18:39 | <annevk> | Domenic: or "I.e., they support being cloned across ..." |
| 18:40 | <Domenic> | Yeah although I prefer to start sentences with "that is" instead of "I.e." |
| 18:40 | <annevk> | sure |
| 18:47 | <TabAtkins> | Domenic: Well Bikeshed has the same issue, so I'm still filing an issue on myself. ^_^ |
| 18:59 | <annevk> | Domenic: thanks so much for all the reviewing |
| 18:59 | <Domenic> | annevk: thanks for doing this!! it's so much nicer. |
| 18:59 | <annevk> | Domenic: ECMAScript prose is rather addictive |
| 19:00 | <Domenic> | You can close https://github.com/whatwg/html/issues/575 and https://github.com/whatwg/html/issues/557 now if you want :) |
| 19:00 | <Domenic> | haha yeah I do like the precision |
| 19:21 | <wanderview_pto> | how many times should I write the same function before I propose a convenience version in the spec? |
| 19:21 | <wanderview_pto> | in this case, a promise-returning service worker wait_for_state() |
| 19:24 | <Domenic> | That sounds like a function very useful for tests and not that useful for other code? |
| 19:27 | <wanderview_pto> | ok |
| 19:30 | <Domenic> | I dunno, are you writing tests or demos? |
| 19:31 | <wanderview_pto> | tests |
| 19:33 | <jsbell> | tests + demos for me; I'd love that too. |
| 19:38 | <jsbell> | wanderview_pto: I think we talked about that very early on, maybe decided not to because the "sw-as-progressive-enhancement" scenario shouldn't use it. |
| 19:39 | <jsbell> | But still, +1 |
| 19:39 | <wanderview_pto> | jsbell: I dunno... I can think of progressive use cases... something like navigator.serviceWorker.register(script).then(swr => swr.installing.waitFor('active')).then(_ => enableOfflineUI()) |
| 19:40 | <wanderview_pto> | you only get the offline UI if you can register a service worker that goes acitve |
| 19:40 | <jsbell> | Right. |
| 19:40 | <wanderview_pto> | although I guess for 'active' we already have the .ready promise |
| 19:40 | <wanderview_pto> | I find the .ready promise a bit weird, tbh |