| 01:55 | <othermaciej> | Is there a reference implementation of the Encoding Standard anywhere? |
| 01:55 | <othermaciej> | I'm trying to make WebKit more compliant but the WPT tests aren't as exhaustive as I would hope for |
| 01:59 | <jgraham> | <insert obvious request to submit more tests here> |
| 02:00 | <jgraham> | othermaciej: I'm not sure about "reference implementation" but https://github.com/hsivonen/encoding_rs is by hsivonen, so probably useful as a reference |
| 02:00 | <jgraham> | Depending on which parts of the standard you care about |
| 02:04 | <othermaciej> | jgraham: I was trying to find out if EUC-KR should actually be EUC-KR or windows-949 |
| 02:04 | <othermaciej> | The spec is clear that the canonical name is EUC-KR |
| 02:07 | <jgraham> | I don't actually know anything much about the spec, sorry |
| 02:11 | <othermaciej> | It seems like other browsers treat it as windows-949, and WebKit has a test that distinguishes, but I do not know how to make an encoding WPT test |
| 02:14 | <othermaciej> | I think I know the correct behavior |
| 02:14 | <othermaciej> | I would like to be a good citizen and add a WPT test but I don't know how |
| 02:14 | <othermaciej> | (yet) |
| 02:15 | <jgraham> | There's really nothing in encoding/legacy-mb-korean/euc-kr/ ? I don't know these tests at all, but there's a lot in there. Dunno how it's generated though |
| 02:16 | <jgraham> | Seems annevk jsbell, Domenic and hsivonen_ have all touched the encoding tests, so one of them might be able to help |
| 02:17 | <jgraham> | Domenic added the euc-kr ones in particular |
| 02:21 | <jgraham> | |
| 02:28 | <Domenic> | There were like 6 outstanding PRs from the i18n folks |
| 02:28 | <Domenic> | I managed to shepherd like 3 of them in |
| 02:28 | <Domenic> | Still several outstanding |
| 02:33 | <Domenic> | othermaciej: I think https://github.com/w3c/web-platform-tests/blob/master/encoding/legacy-mb-korean/euc-kr/euckr-decode.html will test it by virtue of including various other files in https://github.com/w3c/web-platform-tests/tree/master/encoding/legacy-mb-korean/euc-kr and ensuring that they all decode the same (including windows-949) |
| 02:34 | <othermaciej> | Domenic: I see, looks like we just haven't imported that test yet |
| 02:34 | <othermaciej> | which we should |
| 02:37 | <Domenic> | hsivonen_: BTW if you want to give review on the remaining ones in https://github.com/w3c/web-platform-tests/pulls?q=is%3Apr+is%3Aopen+encoding+label%3Awg-i18n_core based on encoding_rs then I can again try to help land them |
| 02:38 | <Domenic> | Hmm I guess I can land the last two as they pass in Firefox 57 which I can take as a +1 from hsivonen_ :) |
| 04:34 | <othermaciej> | I wonder if it's intentional that encodings.json specifies "x-x-big5" as a name for Big5, but not "x-big5" |
| 04:35 | <othermaciej> | spec has it the same way |
| 04:41 | <Domenic> | I wouldn't be surprised |
| 04:41 | <Domenic> | today i discovered cseucpkdfmtjapanese |
| 04:43 | <othermaciej> | Firefox and Chrome match the spec so I'd guess that is good enough for practical use. WebKit has a bunch of extra names for Big5, and a separate Big5-HKSCS encoding. Trying to figure out if it's safe to take it all out. |
| 04:45 | <Domenic> | jsbell (who appears to not be in channel at the moment) is probably our best source of war stories on alignment in Chrome. |
| 04:45 | <Domenic> | Off the top of my head I don't recall anything painful |
| 04:46 | <Domenic> | Some of our use counters for aliases for utf-8 came back at like 0, is the only story I remember |
| 04:52 | <timdream> | othermaciej: this was the bug that merges Big5 with Big5-HKSCS in Gecko, https://bugzilla.mozilla.org/show_bug.cgi?id=912470 , mainly because the non-existence usage of other Big5 variants on the web. Links to other discussion as well — be ware of the rabbit holes. |
| 04:56 | <othermaciej> | timdream: it sounds like Firefox made all Big5 aliases act like Big5-HKCS (but still treating the canonical name as Big5) |
| 04:56 | <timdream> | othermaciej: yes. per Encoding Standard |
| 05:03 | <othermaciej> | Hmm, WPT has a bunch of Big5 tests but sadly most are not on the live version at w3c-test.org yet |
| 05:03 | <othermaciej> | does anyone know how often that gets updated? |
| 06:20 | <othermaciej> | how does WPT decide which HTML files should or should not be run as a test? |
| 06:37 | <annevk> | othermaciej: it’s windows-949; not sure if it’s an exact match |
| 06:38 | <annevk> | othermaciej: the Indexes section has some notes that also explain this |
| 06:38 | <othermaciej> | annevk: that's the conclusion I came to. Had to add a special case to make the canonical name be EUC-KR but the actual encoding be windows-949 |
| 06:39 | <othermaciej> | annevk: some of the legacy-mb- subdirectories under https://github.com/w3c/web-platform-tests/tree/master/encoding seem to have .html files that aren't intended to be run as tests directly but aren't in a resources/ sudir. Do you know if this is intentional? |
| 06:39 | <othermaciej> | (can send a PR if not) |
| 06:40 | <annevk> | “This matches the KS X 1001 standard and the Unified Hangul Code, more commonly known together as Windows Codepage 949.” |
| 06:40 | <othermaciej> | specific example: https://github.com/w3c/web-platform-tests/blob/master/encoding/legacy-mb-japanese/shift_jis/sjis_chars-csshiftjis.html |
| 06:40 | <othermaciej> | I think that is only meant to be loaded as an iframe in an actual test, not run as a test in itself |
| 06:41 | <annevk> | Sorry no, Domenic and Richard Ishida would know about that |
| 06:41 | <annevk> | Maybe that needs cleaning up |
| 06:43 | <Domenic> | othermaciej: yes, lots of HTML files in WPT are iframes. |
| 06:43 | <Domenic> | The WPT manifest will tell you which are tests by scanning to find testharness.js references. |
| 06:44 | <Domenic> | resources/ is just a convention. |
| 06:44 | <othermaciej> | Domenic: is it appropriate to make things follow the resources/ convention if they don't? |
| 06:46 | <Domenic> | I mean, sure, I'd approve a few of those PRs. |
| 06:46 | <Domenic> | Seems a bit cleaner and easier to scan. |
| 06:47 | <othermaciej> | WebKit |
| 06:47 | <othermaciej> | WebKit's copy of WPT seems to not use manifests, it just follows the resources/ convention |
| 06:47 | <othermaciej> | whichi s probably wrong, but fixing that is beyond me |
| 06:48 | <othermaciej> | moving stuff to resources/ is easy enough though |
| 06:49 | <Domenic> | https://wpt.fyi/encoding/legacy-mb-tchinese/big5/ is generated from the manifest so might help |
| 06:51 | <Domenic> | https://wpt.fyi/encoding/legacy-mb-tchinese/big5/big5-enc-ascii.html is a bit troubling hmm, maybe a bad test |
| 07:42 | <annevk> | Ooh so that is how WPT works |
| 07:42 | <annevk> | I vastly prefer the resources/ model |
| 07:54 | <othermaciej> | me too |
| 14:07 | <annevk> | smaug____: looking at https://public.etherpad-mozilla.org/p/composedPath |
| 14:08 | <annevk> | smaug____: "if the target is a closed Shadow DOM root" -> has? |
| 14:08 | <annevk> | smaug____: "if node is assigned" -> s/node/target/? |
| 14:09 | <smaug____> | target |
| 14:09 | <smaug____> | annevk: hmm, has? |
| 14:09 | <smaug____> | no has there |
| 14:09 | <smaug____> | annevk: if it is the root |
| 14:09 | <annevk> | smaug____: okay |
| 14:10 | <smaug____> | I guess ShadowRoot |
| 14:10 | <smaug____> | annevk:oh yeah, I was going to ask if that all looks reasonable |
| 14:11 | <smaug____> | the idea should be simple. Go from window down to the current target, then continue in the current subtree, but bypass closed shadow trees |
| 14:13 | <smaug____> | and doesn't change O() of path creation |
| 14:13 | <smaug____> | nor composedPath |
| 14:14 | <annevk> | smaug____: looks reasonable, I wonder if this is enough for rniwa to review |
| 14:15 | <smaug____> | probably yes |
| 14:15 | <smaug____> | good to get feedback sooner than later |
| 14:22 | <annevk> | smaug____: okay, I'll clean up the text a bit and add it as a comment on your issue |
| 14:22 | <annevk> | smaug____: then if it looks okay I can work on a PR tomorrow |
| 14:22 | <annevk> | smaug____: hopefully someone else can do tests... |
| 14:22 | <smaug____> | I guess I could write tests when implementing it |
| 14:27 | <annevk> | smaug____: could you look at my changes in the etherpad? |
| 14:33 | <annevk> | smaug____: I added some questions and rephrasings as well |
| 14:33 | <smaug____> | looking |
| 14:34 | <smaug____> | annevk: so, we don't create path anywhere else but in concept-event-dispatch, right? |
| 14:36 | <annevk> | smaug____: yeah, we append in step 5, 9.2.2, and 9.4.2 |
| 14:36 | <annevk> | smaug____: then iterate over in 12 and 13 |
| 14:36 | <smaug____> | oh, you mean that |
| 14:37 | <annevk> | I guess this needs to affect step 5 as well |
| 14:37 | <annevk> | So maybe we need to define an "append to path" operation that has these conditionals |
| 14:38 | <smaug____> | right |
| 14:38 | <smaug____> | and/or 'get the parent' would return the relevant flags |
| 14:39 | <annevk> | smaug____: the latter might make it harder for other specifications to use "get the parent" |
| 14:40 | <smaug____> | oh, someone else is using that |
| 14:40 | <smaug____> | surprising |
| 14:40 | <annevk> | smaug____: IDB is supposed to, not sure if it does already |
| 14:40 | <smaug____> | oh, right |
| 14:40 | <smaug____> | well, get the parent would optionally return some flags |
| 14:41 | <annevk> | smaug____: you mean sometimes it returns a single object and sometimes several things? |
| 14:41 | <annevk> | smaug____: hmm |
| 14:42 | <smaug____> | well, it would return a tuple { parent, flagX, flagY} |
| 14:42 | <smaug____> | and flagX and flagY would have some default values |
| 14:43 | <annevk> | smaug____: how would that help btw with step 5? |
| 14:43 | <annevk> | smaug____: get the parent is not yet invoked there |
| 14:44 | <annevk> | smaug____: don't we need to check that target is a closed shadow root there? |
| 14:44 | <smaug____> | yes we need |
| 14:45 | <annevk> | okay |
| 14:45 | <smaug____> | so yeah, need to think a bit how to map the algorithm to the current spec |
| 14:46 | <smaug____> | sorry, I didn't look at the spec when thinking about the setup |
| 14:46 | <annevk> | smaug____: if we do the special "append to path" procedure, would my conditionals work? |
| 14:46 | <annevk> | smaug____: "If target is a shadow root whose mode is "closed", then add a root-of-closed-tree flag to the tuple." |
| 14:46 | <annevk> | smaug____: and "If parent is a slot, parent's root is a shadow root whose mode is "closed", target is assigned, and target's assigned slot is parent, then add a slot-in-closed-tree flag to the tuple." for the second |
| 14:46 | <smaug____> | (I was explicitly not anywhere nearby of any computers when drawing the idea to a paper) |
| 14:47 | <annevk> | oh, with s/parent/target/ I guess |
| 14:47 | <smaug____> | not sure about the last one |
| 14:48 | <annevk> | but yeah, then it falls apart |
| 14:55 | <annevk> | smaug____: put fixed text in the etherpad |
| 14:56 | <smaug____> | annevk: hmm, what if the slot has a child node |
| 14:56 | <smaug____> | and event is dispatched on that |
| 14:56 | <smaug____> | we want to have slot-in-closed-tree flag only when crossing shadow dom boundary |
| 14:57 | <smaug____> | oh, hmm |
| 14:57 | <smaug____> | nm |
| 14:57 | <smaug____> | maybe 'targetOverride's assigned slot is target' somehow captures that |
| 14:59 | <annevk> | I'm not sure |
| 15:00 | <annevk> | This is really quite complicated |
| 15:00 | <smaug____> | well, it isn't |
| 15:00 | <smaug____> | but somehow mapping it to the current spec is |
| 15:01 | <smaug____> | if the get parent returned the flags, and step 5 explicitly checked shadowroot, that should be enough |
| 15:02 | <smaug____> | I wonder, would it help writing the spec if the flag wasn't in the slot, but the assigned node |
| 15:04 | <annevk> | smaug____: the question about "what if slot has a child node" is also relevant for your non-spec explanation |
| 15:05 | <smaug____> | how so? |
| 15:05 | <smaug____> | slot is then treated just like any other node in shadow dom |
| 15:05 | <smaug____> | or are you thinking some different case? |
| 15:06 | <annevk> | smaug____: all I did was make what you wrote more explicit, so I don't really see where you think there's a difference |
| 15:07 | <smaug____> | annevk: maybe you didn't change. As I said, perhaps 'targetOverride's assigned slot is target' takes care of that |
| 15:07 | <smaug____> | just a bit hard to read |
| 15:07 | <smaug____> | (sorry, I'm multitasking) |
| 15:09 | <smaug____> | I see: "Append (parent, target, relatedTarget) to event’s path. " |
| 15:09 | <smaug____> | that should work... and then, what happens with step 5 |
| 15:10 | <annevk> | smaug____: we'd replace "Append" everywhere with "Append to path" that has these two conditionals |
| 15:11 | <smaug____> | ok, yeah, that tagetOverride stuff should be fine |
| 15:11 | <smaug____> | doesn't matter in step 5 |
| 15:12 | <annevk> | smaug____: it'll just no-op in step 5 and since we do need the first conditional it seems fine to reuse the code path |
| 15:12 | <annevk> | smaug____: okay, thanks for having another look, I'll add this to the issue |
| 15:16 | <annevk> | smaug____: https://github.com/whatwg/dom/issues/525#issuecomment-342179455 |
| 15:17 | <smaug____> | thanks |
| 15:17 | <smaug____> | hopefully rniwa has time to take a look |
| 18:21 | <annevk> | othermaciej: FWIW, Chromium has made an effort to align with Encoding; but if you’re only interested in labels that is less interesting |
| 18:21 | <annevk> | othermaciej: note that if you only care about labels for now you prolly want to leave big5 alone |
| 18:22 | <othermaciej> | annevk: eventually we'll fix the encoder behavior too, I'm just trying to fix labels first |
| 18:22 | <othermaciej> | we have a whole extra encoder implementation that's non-ICU which is only used to implement nonstandard encodings |
| 18:22 | <othermaciej> | I would like to get rid of it |
| 18:23 | <othermaciej> | for Big5 it seems likely I'll need a custom encoder just to be able to align on labels |
| 18:27 | <annevk> | othermaciej: I think Chromium applies some kind of patch, though I have not looked recently |
| 18:28 | <annevk> | othermaciej: and with ICU deferring to the Encoding Standard for UTF-8 perhaps some will be upstreamed |
| 18:30 | <othermaciej> | annevk: I think ICU has a sort of generic mechanism for defining encodings (.ucm files), so it should be at least possible to add the web variants of particular encodings |
| 19:28 | <gsnedders> | annevk: Blink was meant to be moving away from their ICU fork, fwiw |
| 21:19 | <annevk> | gsnedders: seems hard unless they upstream all patches somehow |
| 21:25 | <jgraham> | annevk: Depends how you interpret the sentene |
| 21:25 | <jgraham> | *sentence |
| 21:26 | <jgraham> | If it implies "move away from ICU" |
| 21:26 | <jgraham> | Then upstreaming isn't required |
| 21:27 | <annevk> | That would be exciting, but JS i18n makes that unlikely |
| 21:33 | <gsnedders> | annevk: it's easy when you rewrite the reason for all the patches :) |
| 21:33 | <gsnedders> | annevk: AIUI, it's inline layout that they have most patches for, and rewriting that avoids the need for them |
| 21:35 | <annevk> | Would not work for the encoding bits afaik |