| 01:28 | <heycam> | TabAtkins, "style sheet!!as conformance class, ??" appears in the css-variables index |
| 08:37 | <drott> | hallvors: hi |
| 08:38 | <hallvors> | hi :) |
| 08:39 | <drott> | thanks for your review! |
| 08:39 | <drott> | wanted to explain a few things about how the organisation of the tests into files etc. ended up like this, and hear from you where we should go with this |
| 08:41 | <drott> | so - as you realized, tests originally came from FF, where they are run all in one |
| 08:41 | <drott> | or in two parts, one for workers, one for document context |
| 08:43 | <drott> | but the test execution is very long, ~3 minutes or so |
| 08:43 | <drott> | webkit layouttests don't support this long execution time |
| 08:43 | <drott> | so i extracted the logic into the xhrtimeout-tests.js (IIRC) - what you called the framework code |
| 08:43 | <drott> | and run it from separate files |
| 08:44 | <drott> | now, there is this groupFromFilename() function that does as the name says, extracts the group of tests that should be run in these split files |
| 08:45 | <drott> | if i understood your feedback correctly, you'd like the code for each file that runs a group of tests to be more close to that file |
| 08:46 | <drott> | but several test.html files share code in the framework, since they each run only a few of a group of similar tests |
| 08:47 | <drott> | so if i unroll/fold-out the groupFromFilename() and put the group to be executed into the files explicitly, we have duplicated the name from the filename to the content of the test - i don't know how much we gain by that |
| 08:48 | <drott> | and the second thing, separating the code out, making it less "frameworky" duplicates code too, since each individual test file would need a copy of what's in the xhrtimeout-tests.js |
| 09:00 | <hallvors> | drott: sorry for not responding, I had some phone calls.. |
| 09:02 | <hallvors> | for most tests, I'd suggest splitting the code into a "common.js" file that all test cases load, and a "testNNN.js" file that has the code that executes the actual test |
| 09:03 | <hallvors> | now, I'm not very familiar with workers |
| 09:03 | <hallvors> | can a worker "include" or "require" another .js file? |
| 09:03 | <hallvors> | so do something like this at the top of a test: if(typeof document == 'undefined') require('common.js') ? |
| 09:04 | <drott> | good question re workers and require |
| 09:07 | <jgraham> | http://www.whatwg.org/specs/web-apps/current-work/multipage/workers.html#importing-scripts-and-libraries |
| 09:07 | <drott> | well, so the xhrtimeout-tests.js contains the logic for callbacks etc. - actually running the tests - at the top |
| 09:08 | <drott> | and a lower section that defines the groups |
| 09:08 | <drott> | and which tests to run |
| 09:08 | <hallvors> | We have importScripts(): http://www.whatwg.org/specs/web-apps/current-work/multipage/workers.html#dom-workerglobalscope-importscripts |
| 09:08 | <drott> | so most of it is like you suggest "common.js" - the rest would be per-test or per-test group |
| 09:09 | <hallvors> | I think that approach would be simpler for both developers debugging some problem exposed by the tests, and for future maintenance of the tests. |
| 09:10 | <hallvors> | (I was also somewhat concerned about whether the complexity would make tests unstable, but I haven't had time to really investigate that so I don't know.) |
| 09:10 | <hallvors> | Have you run these tests systematically over time somewhere? |
| 09:10 | <drott> | they run as part of the webkit layout tests several tens of times per day |
| 09:10 | <hallvors> | OK, that sounds good |
| 09:11 | <jgraham> | To be fair, so do a lot of unstable tests :) |
| 09:12 | <hallvors> | heh |
| 09:12 | <drott> | jgraham: i don't disagree with that - but these ones here are not flaky, they've proven quite reliable |
| 09:12 | <hallvors> | is there some place to review the results? |
| 09:12 | <hallvors> | anyway, given that I'll only argue that refactoring is good for debugging and maintenance ;-) |
| 09:12 | <jgraham> | drott: Good to know :) |
| 09:15 | <drott> | hallvors: the webkit waterfall has test execution logs, which marks them as "Passed", which means, they match the expected result, which has a few PASS lines matching the output of testharness.js |
| 09:15 | <drott> | here's for example a log of the http://build.webkit.org/builders/GTK%20Linux%2064-bit%20Release%20WK2%20%28Tests%29/builds/6753/steps/layout-test/logs/stdio |
| 09:15 | <drott> | GTK build and test run |
| 09:15 | <drott> | you see lines like http/tests/xmlhttprequest/timeout/xmlhttprequest-timeout-simple.html passed |
| 09:16 | <drott> | hallvors: debugging might be a bit easier since you don't have to figure out the launch logic |
| 09:17 | <drott> | hallvors: but maintenance i actually find easier with the approach of just having thin files that you don't need to touch - instead, the logic is in the "framework", in one place |
| 09:20 | <hallvors> | well - imagine that you're not familiar with the setup, and want to add a new test - it's significantly harder if you have to figure out this "launch logic" and then search through the files to find groupFromLocation() and then figure out where it gets its data from.. ;-) |
| 09:21 | <hallvors> | It's also confusing while trying to understand the setup that the RequestTracker constructor is called so many times, because it defines all the objects for each test, even those that won't be used in this test but will be used in others. |
| 09:23 | <drott> | hallvors: okay, I'll see what I can do |
| 09:23 | <drott> | hallvors: again, thanks for the feedback |
| 09:24 | <hallvors> | It shouldn't be too hard (I hope) - basically copy the code that defines the arrays in TestRequestGroups into separate "testNNN.js" files corresponding to the HTML files.. |
| 09:24 | <hallvors> | add an importScripts statement for the worker case |
| 09:24 | <drott> | hallvors: maybe along with the requestTracker (and other objects) that each of them needs |
| 09:24 | <hallvors> | no, the RequestTracker definition should be in common.js |
| 09:25 | <drott> | the class |
| 09:25 | <hallvors> | :) |
| 09:25 | <hallvors> | IMHO |
| 09:25 | <drott> | but the instances could move into NN.js |
| 09:25 | <hallvors> | exactly |
| 09:25 | <drott> | alright |
| 09:27 | <hallvors> | so somebody not familiar with the tests looks at timeout-twice.html, sees link to timeout-twice.js, and gets the three RequestTracker instance definitions instantly |
| 09:29 | <hallvors> | and thank you for listening and discussing this feedback :) |
| 09:30 | <hallvors> | hello nimbu :) |
| 09:46 | <ambv> | gsnedders: Where do I put this example? I want to fix this -> http://www.bpaste.net/show/OHlEpp27lbdawO2thfbR/ |
| 09:47 | <gsnedders> | WTF. When did we break that? |
| 09:47 | gsnedders | sighs |
| 09:48 | <ambv> | Definitely a 1.0 blocker ;-) |
| 09:51 | <ambv> | gsnedders: so, where does the example go? http://www.bpaste.net/show/clrC8KkPoLQ385vnwDEN/ |
| 10:06 | <jgraham> | drott: Sorry for the spam |
| 10:07 | <ambv> | jgraham: hello there, James! |
| 10:20 | <ambv> | jgraham: gsnedders: can you scroll through the slides to see whether everything's fine? https://dl.dropboxusercontent.com/u/4830253/html5lib.pdf |
| 10:26 | <gsnedders> | ambv: You just need to change the test harness so it doesn't outright ignore tests the cause DataLossWarning. Like, they should have a pass condition of "does not raise an exception". |
| 10:26 | <gsnedders> | ambv: Instead of the current situation of "DataLossWarning? Well, we'll ignore you entirely" |
| 10:27 | <hallvors> | Implementor comments sought: is it hard to allow XHR overrideMimeType() to fix content type / encoding info in HEADERS_RECEIVED mode? |
| 10:28 | <gsnedders> | ambv: I'm always dubious about using affiliations for open source projects, but if you do, get the (current) Opera logo! http://business.opera.com/press/resources has it in all kinds of formats. |
| 10:28 | <gsnedders> | ambv: Also has several vector formats, which is probably a good idea |
| 10:29 | <jgraham> | Yeah, I don't really know to what extent this is an "Opera" thing |
| 10:29 | <jgraham> | Somehow I missed the fact that you're using a four-year-old logo |
| 10:30 | <gsnedders> | I mean, both of us were working on it pre-Opera |
| 10:30 | <jgraham> | Could be nice to have an example of using html5lib other than a blurry screenshot of the documentation |
| 10:30 | <jgraham> | Since you have lots of examples of using !html5lib |
| 10:31 | <ambv> | jgraham: haha, sure. |
| 10:32 | <jgraham> | On the lxml slide "It's a C extension" should probably be "lxml is a C extension" |
| 10:32 | <ambv> | gsnedders: raising on DataLossWarning breaks 98 tests currently |
| 10:32 | <gsnedders> | ambv: Yay. |
| 10:34 | <jgraham> | That's not surprising; it was used to hide things that simply aren't supported in some treebuilders iirc |
| 10:34 | <jgraham> | gsnedders: https://github.com/jsocol/bleach/blob/master/bleach/sanitizer.py |
| 10:35 | <ambv> | gsnedders: Maybe a more pragmatic approach for now would be to fix the direct issue (fun fact: it works on 3.2+ and PyPy) |
| 10:38 | <gsnedders> | jgraham: If something didn't get lost in communication, 98 tests raise exceptions (not warnings) whilewarning DataLossWarning. |
| 10:39 | <gsnedders> | jgraham: We should never throw an exception, we should just not check what tree we get |
| 10:41 | <ambv> | tests/test_parser.py catch DataLossWarnings as exceptions and ignore them which means such tests don't ever run anything useful |
| 10:45 | <ambv> | gsnedders: duh, the bug is really basic |
| 10:46 | <ambv> | unicode_literals means that the second argument in hex(123).rjust(5, "0") is unicode, should be bytes |
| 10:46 | <gsnedders> | I was guessing it was something stupid. |
| 10:47 | <gsnedders> | Yay, another case where we want the native str type, whatever that is |
| 10:47 | <ambv> | I'll just fix that up, but we lack a test for escapeChar and friends |
| 10:48 | <gsnedders> | Would "%05x" not do the right thing? |
| 10:48 | <gsnedders> | Which seems simpler than hacking around with string types. |
| 10:48 | <ambv> | yeah, I was going |
| 10:48 | <ambv> | to try that |
| 10:49 | <ambv> | but "%05x" is still unicode with unicode literals ;-) |
| 10:49 | <gsnedders> | Do we not want that? |
| 10:50 | <gsnedders> | ambv: Everything in/out of there should be unicode |
| 10:51 | <ambv> | replacement = ("U%05x" % (ord(char),)).upper() |
| 10:52 | <gsnedders> | Why not just %05X? |
| 10:53 | <ambv> | because I forgot it exists. ha. |
| 10:54 | <ambv> | but I want a test for that breaking example I pasted you just for regressions. |
| 10:54 | <gsnedders> | I expect that's the only issue breaking all the DataLossWarning hidden problems? |
| 10:54 | <ambv> | the only one we're aware of at the moment |
| 10:54 | <ambv> | ;-) |
| 10:55 | <gsnedders> | Would be worth trying to see how much effort it is to fix any exception raised, currently hidden behind DataLossWarning. |
| 10:55 | <gsnedders> | Because we have tests that will hit that exact case. |
| 10:58 | <ambv> | Yes, but let me do that after 1.0b1 |
| 10:59 | gsnedders | has Flake fail on master, and wonders how that never showed up before |
| 10:59 | <ambv> | we should have flake invocations converted to Bash scripts so that I can connect them to tox |
| 11:00 | <ambv> | this way I'll be running them everytime locally |
| 11:00 | <gsnedders> | Oh, merge broke it somehow. :( |
| 11:04 | <gsnedders> | ambv: That's been on my to-do list for a while :P |
| 11:05 | <ambv> | Sure thing. |
| 11:05 | <ambv> | Now only a test to check we don't have a regression. It's tricky ;-) |
| 11:05 | <ambv> | http://www.bpaste.net/show/Na6TZMB4H6PkT1Wype11/ |
| 11:06 | <gsnedders> | jgraham: https://critic.hoppipolla.co.uk/5056ff57?review=106 |
| 11:06 | <gsnedders> | jgraham: (it's a single CHANGELOG entry) |
| 11:09 | gsnedders | wonders how tests have got so much slower |
| 11:13 | <gsnedders> | Nah, I'm just imagining them being quicker :P |
| 11:20 | <ambv> | PULLDOM Y U SO MEAN! http://www.bpaste.net/show/8ePI4jZLClouWS6Ua106/ |
| 11:22 | <darobin> | zcorpan: you there? |
| 11:22 | <zcorpan> | darobin: yes |
| 11:22 | <darobin> | zcorpan: have you made a publication snapshot of the diffs doc? |
| 11:22 | <zcorpan> | darobin: no, but i can do it now |
| 11:22 | <darobin> | I can't see to see it in GH or CVS |
| 11:22 | <darobin> | that would be wunderbar |
| 11:22 | <zcorpan> | what's the date? |
| 11:22 | <darobin> | 2805 |
| 11:23 | <zcorpan> | heh, and people were upset about 2022! |
| 11:23 | <darobin> | lol |
| 11:30 | <hsivonen> | what does Google Play Music All Access use for playing music on the Web? Chrome-only? Cross-browser? |
| 11:32 | <zcorpan> | i don't recall what command i should use to specify a date for make... but i can fix it up manually |
| 11:34 | <zcorpan> | figured it out |
| 11:34 | <zcorpan> | make tr PUBDATE="28 May 2013" |
| 11:41 | <zcorpan> | darobin: https://rawgithub.com/whatwg/html-differences/master/TR.html |
| 11:43 | <darobin> | zcorpan: you're a star, thanks! |
| 11:43 | <darobin> | I wish all editors were like you :) |
| 11:45 | <zcorpan> | i try not to imagine what the implications of that would be :-P |
| 11:46 | <darobin> | zcorpan: well, except for the bit where you used <hgroup> :) |
| 11:49 | <zcorpan> | oh, is that not OK? |
| 11:49 | <zcorpan> | are w3c specs only allowed to use w3c html5? |
| 11:52 | <darobin> | zcorpan: yeah, they use what the validator says is good |
| 11:52 | <darobin> | but that's not a big deal, I just replaced it |
| 11:53 | <zcorpan> | i'll fix it anyway |
| 11:53 | <zcorpan> | i see the w3c html spec uses <p> for the subheading |
| 11:55 | <gsnedders> | ambv: Will push out 1.0b1 some time after lunch |
| 11:55 | <gsnedders> | Speaking of which, to lunch! |
| 11:55 | <ambv> | no no no! |
| 11:56 | <ambv> | gsnedders: push it with this -> https://github.com/html5lib/html5lib-python/pull/51 |
| 11:59 | <ambv> | That being said, bon appetit! |
| 12:01 | <zcorpan> | darobin: fixed |
| 12:01 | <gsnedders> | ambv: Follow style elsewhere and don't use a tuple with a single item (i.e., use `"…" % ord(…)`) |
| 12:01 | <darobin> | zcorpan: super, thanks! |
| 12:01 | <darobin> | zcorpan: using <p> for the subheading is a very recent addition |
| 12:02 | <darobin> | it's from Steve's work on replacing hgroup |
| 12:02 | <gsnedders> | ambv: And if all tests pass with that with DataLossWarning not hiding exceptions, I'd much rather change test_parser.py than adding another test. |
| 12:02 | <darobin> | I think only the HTML spec uses that at the moment |
| 12:02 | <zcorpan> | darobin: yeah |
| 12:03 | <ambv> | gsnedders: I will change the styling as you wish. Just note that in general leaving out the tuple is incorrect (what if your single element ends up being a tuple?). |
| 12:04 | <ambv> | gsnedders: as for the test, putting this in html5lib-tests breaks PullDOM as seen here -> http://www.bpaste.net/show/8ePI4jZLClouWS6Ua106/ and I don't want to deal with that just now |
| 13:57 | <gsnedders> | ambv: Yeah, I know that it's wrong in general. |
| 13:57 | <ambv> | The tests failed on Travis, lemme just fix it up. |
| 13:59 | <ambv> | gsnedders: can I easily commit stuff on submodules? I mean, I have a clone of a submodule but I changed some test data in the html5lib working copy to see if they work at all |
| 13:59 | <ambv> | what's your workflow for that case? |
| 14:04 | <gsnedders> | ambv: Is there a nice submodule workflow given GitHub's permission mode? |
| 14:05 | <ambv> | this is why I'm asking. I hardly ever use submodules because of that. |
| 14:06 | <hallvors> | annevk: https://github.com/whatwg/xhr/pull/6 |
| 14:15 | <gsnedders> | ambv: If we change the submodule to be GitHub over HTTP you can checkout master in there and commit/push in it |
| 14:15 | <gsnedders> | Which is probably better |
| 14:16 | <ambv> | +1 |
| 14:16 | <ambv> | gsnedders: in related news, I fixed the failing tests |
| 14:29 | <ambv> | gsnedders: and Travis agrees, it works. https://github.com/html5lib/html5lib-python/pull/51 |
| 14:31 | <jgraham> | ambv: Does your test actually check that you get the right tree? |
| 14:32 | <ambv> | jgraham: for that I have another test that goes in html5lib-tests |
| 14:32 | <ambv> | because I want to test whether the tree is good regardless of the builder |
| 14:33 | <ambv> | currently it fails for PullDOM (where the "xml:" prefix is discarded) which is why I don't want to have it in just now |
| 14:33 | <gsnedders> | jgraham: sof's bug seems to be hidden by DataLossWarning currently, BTW |
| 14:36 | <ambv> | jgraham: here's the test - https://github.com/html5lib/html5lib-tests/pull/6 we'll merge it when I fix PullDOM but this will be after DjangoCon |
| 14:36 | <ambv> | (unless you do it first, of course :)) |
| 14:36 | <jgraham> | ambv: OK, fine, might want a comment to that effect |
| 14:37 | <gsnedders> | Should it not be the ihatexml name? |
| 14:37 | <gsnedders> | i.e., the infoset coerction name? |
| 14:38 | <ambv> | gsnedders: what are you referring to? |
| 14:39 | <annevk> | "So, largely, implementations don't let you do cross-origin XHR auth at all.." I recall saying exactly that. Good it's backed up by tests though. |
| 14:39 | <gsnedders> | ambv: https://github.com/html5lib/html5lib-tests/blob/master/tree-construction/tests14.dat#L34 seems to test xml:lang in HTML already? |
| 14:39 | <ambv> | so it has to be cancelled out by catching DataLossWarning |
| 14:40 | <gsnedders> | I don't think PullDOM can repesent xml:lang as an attribute? |
| 14:41 | <ambv> | what should we do with it then? skip the test for PullDOM? make it an expected failure? the test autogeneration doesn't seem to support this now. |
| 14:45 | <gsnedders> | ambv: Where did you add the catching warnings? test_parser.py? |
| 14:45 | <gsnedders> | Because with your fix I'm only seeing six more failures, and that tempts me to fix them all now. |
| 14:46 | <ambv> | in the test itself |
| 14:46 | <ambv> | https://github.com/ambv/html5lib-python/commit/add2c558a1c27e8237e2501c3850c178966ad0ee |
| 14:46 | <ambv> | this way when the warning goes away in the future, we will have a failure and know that something changed |
| 14:58 | <gsnedders> | jgraham: ihateihatexml |
| 14:59 | <jgraham> | We all do. But less than we hate xml |
| 15:00 | <jgraham> | Which didn't have to have all these stupid limitations |
| 15:17 | <ambv> | gsnedders: jgraham: I'm doing the LT tomorrow at 5:00 PM CEST, it would be great if 1.0b1 is up by then. |
| 15:17 | <ambv> | tell me if you need anything else |
| 15:30 | <gsnedders> | jgraham: What's the best approach for coercing public and system ids? |
| 15:30 | <gsnedders> | jgraham: Note the allowed set of chars differs between single and double quoted strings |
| 15:30 | <Ms2ger> | gsnedders, you called yesterday? |
| 15:30 | <gsnedders> | Ms2ger: Get my email? |
| 15:30 | <Ms2ger> | Yep |
| 15:30 | <Ms2ger> | Thanks |
| 15:34 | <jgraham> | gsnedders: In that " isn't allowed if it's double quoted and ' isn't allowed if it's single quoted? |
| 15:39 | <gsnedders> | jgraham: Yes. |
| 15:39 | <tantek> | aaronpk I heard about the Google's "email actions" but hadn't seen the details til that link |
| 15:39 | <tantek> | oops wrong channel |
| 15:50 | <gsnedders> | jgraham: Well, a PubidLiteral can never contain " |
| 15:51 | <gsnedders> | jgraham: But ' is allowed when double quoted, but not when single quoted. |
| 15:52 | <jgraham> | I don't remember how doctype and lxml work off the top of my head. I have recollections of a terrible hack |
| 15:53 | <gsnedders> | jgraham: We serialize the doctype as XML then parse it |
| 15:54 | <jgraham> | I was hoping that I had misremembered :( |
| 15:55 | <jgraham> | So always double quoting and escaping any remaining invalid cahracters in the normal way should be enough? |
| 15:55 | <gsnedders> | jgraham: Escaping meaning what? |
| 15:55 | <gsnedders> | jgraham: You cannot escape characters in pubid/sysid |
| 15:56 | <jgraham> | The U+12345 stuff |
| 15:56 | <gsnedders> | Right. |
| 15:57 | <jgraham> | It's either that or drop the characters I guess |
| 15:57 | <jgraham> | I think this is a little better |
| 15:58 | <gsnedders> | Or return it quoted from ihatexml |
| 15:58 | <gsnedders> | Which at least allows it somewhat. |
| 15:58 | <jgraham> | Return waht quoted? |
| 16:09 | <gsnedders> | jgraham: Have coercePubid(value) return the quoted value. |
| 16:16 | <ambv> | gsnedders: boom. https://github.com/html5lib/html5lib-python/pull/52 |
| 16:27 | <gsnedders> | ambv: There's an identical commit on the dom2sax PR :) |
| 16:27 | <ambv> | dang |
| 17:14 | <TabAtkins> | heycam|away: Yeah, I need to just remove those dfns from my boilerplate. They don't seem to accomplish anything, and they clutter up the index. |
| 17:14 | <TabAtkins> | (And are the only instance of "sub-terms" in most specs, which is what the !! thing is about.) |
| 18:15 | <ambv> | gsnedders: in unrelated news, sanitizer fails for a document like "<p><script>alert('Boo!')</script>Surprise!" |
| 18:15 | <ambv> | serialization breaks with TypeError: unhashable type: 'slice' on line 183 of html5lib/sanitizer.py |
| 18:15 | <ambv> | will look into it later |
| 20:40 | <mrbkap> | dglazkov: ping? |