mozilla :: #servo

15 Mar 2017
00:00Manishearthheycam|away: cssproperties list updated
00:00xidornbholley: oh, hmmm, yeah, SpecifiedUrl contains a resolved ServoUrl
00:01xidornbholley: I'm not sure whether that is necessary for stylo...
00:05Manishearthxidorn: -moz-binding doesn't like the original url being used
00:05Manishearthall other properties are fine
00:10xidornManishearth: that suddenly makes me wonder... so in gecko and servo, we resolve url during parsing, but in stylo, we resolve url after computing?
00:10xidornwhich sounds scary...
00:13Manishearthxidorn: uhhhh idk
00:13Manishearthxidorn: I'm not very clear on how this works
00:13bholleyxidorn: that sounds like another good reason to eagerly create nsStandardURL
00:13Manishearthwe used to do it during parsing
00:13Manishearththen we defered it a bit for reasons
00:13bholleyManishearth: we created gecko urls during parsing?
00:13Manishearththen the image stuff happened and we suddenly had two urls in one
00:13Manishearthbholley: no servo
00:13Manishearthwe have this weird thing where we go from original -> resolved -> nsstandard
00:14bholleyok, I'll file a bug for all this URL stuff
00:14Manishearthcool
00:14bholleyManishearth: another topic - we spend a lot of time doubling the vec in parse_property_declaration_list. Seems like we want a large stack-allocated array, and then vecify at the end
00:14xidornI think heycam implemented the computation-time resolving in bug 1288302
00:14firebothttps://bugzil.la/1288302 FIXED, cam@mcc.id.au Stylo: implement support for background-image: url()
00:15bholleyManishearth: is there a standard rust data structure to do something like that, or should I roll my own?
00:15xidorn(which was reviewed by me... and I didn't notice this issue :/
00:15edunhamManishearth: re https://bugzilla.mozilla.org/show_bug.cgi?id=1347259, I'd also prefer to have a more descriptive email than just "Bors" because that name is getting almost dangerously overloaded in the greater Rust/Servo ecosystem
00:15firebotBug 1347259 NEW, mozillamarcia.knous@gmail.com Commit access (Level 1) for bot
00:15Manishearthedunham: sure
00:15Manishearthhgbot wfm
00:16Manishearthbholley: smallvec I guess?
00:16Manishearthnot really
00:18xidornbholley: yeah, I think we should eagerly create gecko's url object and pass that to style struct
00:18Manishearthbholley: I feel like the doubling cost is due to copies, not realloc
00:18Manishearthwe still have 40 byte properties
00:18Manishearthwe can cut down on that
00:19xidornbholley: so that we can also remove the complexity for computation-time resolving...
00:19Manishearthbholley: when creating a vec of size n, you'd have n/2 + n/4 + n/8 + ... = n copies *anyway*
00:19Manishearthso stack-allocate-then-copy doesn't really avoid copies
00:19Manishearthit just avoids reallocs
00:19xidornand we can probably even remove the needed for thread-safe url wrapper?
00:19bholleyManishearth: yes, but it's also l1
00:19Manishearthbholley: hm
00:20Manishearthkinda feel like any large array of consequence will blow L1 anyway
00:20Manishearthagain, 40byte elements
00:20Manishearththat's one and a half elements per cache line
00:21KiChjangwait, it takes 2 hours to test saltfs?
00:22ManishearthKiChjang: it does a vagrant and everything
00:22Manishearthso yeah
00:22KiChjangdarn, i thought it got stuck
00:23bholleyManishearth: hm, so I guess overall we spend twice as much time memmoving as we do mallocing
00:23bholleyManishearth: I wonder if we should shrink PropertyDeclarations more
00:24bholleyManishearth: what are the cliffs we're looking at if we do that?
00:25Manishearthbholley: cliffs?
00:25bholleyManishearth: i.e. what has to spill?
00:25Manishearthbholley: a lot of properties
00:26Manishearthif we can get coord values down to 8 bytes that would be good
00:26Manishearthi think that's doable. maybe not
00:26Manishearthor they might be already
00:27bholleyManishearth: coord values?
00:27bholleyManishearth: you mean the stuff that maps to nscoord?
00:27Manishearthbholley: LoP etc
00:27Manishearthyeah
00:27Manishearthbholley: fwiw gecko is quite liberal on allocs here
00:27Manishearththe only thing that isn't alloced is stuff that's "number + unit"
00:27Manishearthi.e. coord values (lengths etc)
00:27Manishearththat's like 50% of the properties
00:28Manishearthif we can see why they're larger than 8, and shrink them, that would be great
00:28Manishearthwell, maybe 16 hold on
00:29Manishearthcolors, ints, floats, and ptrs
00:30Manishearth2bytes for the unit, 8 for the value, so 10 total
00:30bholleyManishearth: this is Gecko?
00:30Manishearthbholley: yes
00:30bholleyManishearth: gecko does some nominal refcounting IIRC
00:30bholleybut most of it is allocs
00:31bholleyManishearth: anyway - do we have a list or visualization of what would need to spill to shrink further?
00:31Manishearthmost of our lengths are 16, which is okay i guess
00:31Manishearthbholley: https://github.com/servo/servo/pull/15065#issuecomment-273036297
00:31crowbotPR #15065: Use Box<CalcLengthOrPercentage> in specified values to avoid bloating inline sizes - https://github.com/servo/servo/pull/15065
00:31Manishearththis is pre-boxing
00:31Manishearthi don&#39;t have post-box numbers
00:32bholleyManishearth: seems like we should shrink to 32
00:32bholleyManishearth: we could maybe add a second enum for color
00:32bholleyManishearth: an RGB-valued enum
00:32bholleyManishearth: to avoid always storing the alpha
00:33bholleyManishearth: is there any reason those numbers wouldn&#39;t be accurate post-boxing?
00:33avadacatavrastandups: xow test still works (by works i mean fails epically, but runs). tracing some toy examples to make sure i understand what needs to be done
00:33standupsOk, submitted #43716 for https://www.standu.ps/user/avadacatavra/
00:33bholleyManishearth: (I also don&#39;t understand why color is 40bytes but border-top-color is 48 bytes...)
00:34Manishearthbholley: well, post-boxing the top half would be chopped off
00:34bholleyManishearth: right - I just wanted to make sure that nothing else got bigger as a result
00:34Manishearthbholley: yeah it wouldn&#39;t
00:35bholleyManishearth: any idea why those colors are different sizes?
00:35Manishearthnope
00:35Manishearthit&#39;s possible that they were different types back then
00:36bholleyManishearth: how did you generate that list?
00:36Manishearthbholley: uhhhhhhhhh :)
00:36ManishearthI think. i makogenerated some printfs
00:36Manishearthand stuck them somewhere
00:36bholleyok
00:37Manishearthi can do the same
01:21heycamManishearth: thanks
01:53Manishearthbholley: https://gist.github.com/Manishearth/a2a7c5ef68151f18026f7a706054e67f
02:17bholleyManishearth: that&#39;s up to date?
02:18bholleyManishearth: if so we should definitely shrink to 32 and pin it there with the size_of tests
02:19Manishearthbholley: YES
02:20Manishearther
02:20Manishearthyes
02:20Manishearthcapslock
02:20bholleyManishearth: ok great - do you want to write the patch, or should I?
02:22Manishearthbholley: go ahead, busy for now
02:22bholleyManishearth: k
02:22Manishearthprobably could be an easy bug
02:22bholleyManishearth: pastebin me the patch you used?
02:23Manishearthbholley: https://manishearth.pastebin.mozilla.org/8982071
02:23Manishearthit&#39;s going to print it a million times, so you need the start/end markers to cut it out
02:23bholleyManishearth: k
03:20xidornheycam: are fill and stroke normal css properties or just svg attributes mapped into css?
03:21heycamxidorn: well, both. they have presentation attributes which are mapped into css properties, and those css properties can be set in style sheets
03:22xidornheycam: actually I want to know where is the syntax of these properties defined?
03:22xidornheycam: is that the svg spec?
03:22heycamxidorn: yes
03:22heycamxidorn: https://svgwg.org/svg2-draft/painting.html#DataTypePaint
03:23xidornheycam: it doesn&#39;t seem to match how we parse it
03:23heycamthough we don&#39;t implement the child values
03:23xidornchild values?
03:23heycamchild and child(<integer>)
03:23heycam(which were meant to be references to child elements that are paint server elements)
03:24xidornoh, okay, it seems svg2 has a different syntax than svg1.1
03:24xidornfor this
03:25xidornheycam: it seems we don&#39;t handle the fall-back color for <url> value properly in both gecko and servo
03:26heycamxidorn: it might be worth checking what other browsers do for the fallback color
03:28xidornthe code seems to be reasonable... but the result doesn&#39;t... hmmm
03:28xidornI need to dig a bit
03:29xidornoh, nope, the code isn&#39;t
03:29heycam:)
03:29* bz wonders what the chance is of emilio still being awake
03:30xidornbz: he may wake up in several hours :)
03:31bzwell, right
03:31bzsomething is really fishy with vw units
03:31bzlike &quot;behave differently in opt and debug builds&quot; fishy
03:34* xidorn guesses there is some timing-dependent issue
03:34bzyeah
03:34bzwell
03:34bzthere&#39;s the &quot;they don&#39;t update on viewport resize&quot; issue, I bet
03:34bzI expect that&#39;s it, actually
03:35* bz marks his test skip-if(stylo)
03:36bzhrm, no, they do update....
03:36bzwhat the heck
03:36bzThis is really bizarre
03:48Manishearthheycam: xidorn we don&#39;t parse the full svg2 syntax
03:48Manishearthwe parse some syntax in between
04:17* xidorn doesn&#39;t really want to file servo issue for non-standard stuff :(
04:21wafflesxidorn, we can file all props that have been implemented in gecko, right?
04:21xidornwaffles: we probably should... I just feel sad when I see them :/
04:21waffleshe
05:09bholleyManishearth: hm, marker-end appears to be 24 bytes on my machine
05:11bholleyManishearth: oh I see - the size changes between stylo and servo
06:18wafflesbholley, not sure whether the those tests will be ran in servo
06:31bholleywaffles: alternative approaches welcome
06:31* bholley sleeps
06:31wafflesbholley, yes, looking :)
06:31wafflesgood night :)
06:56xidorndownloading rustc seems to be slow today :/
07:46SimonSapinbholley, Manishearth: https://github.com/servo/servo/blob/master/tests/unit/style/size_of.rs#L29 has data about the size of each variant of PropertyDeclaration
07:47SimonSapina couple weeks ago I wrote a couple failing assert_eq!s in that function to see how many are above/below various thresholds
07:47SimonSapin(I didnt keep that code)
07:49SimonSapinwe can certainly box more properties to cut the size to 32, 24, or even 16 bytes for PropertyDeclaration
07:49SimonSapinthough with alignment the latter only leaves a single word for the value
07:49SimonSapin(this is all assuming 64 bit)
09:05noxheycam: Why didn&#39;t you just r+ https://github.com/servo/servo/pull/15956#issuecomment-286668991?
09:05crowbotPR #15956: Fix -moz-user-select: tri-state - https://github.com/servo/servo/pull/15956
09:06noxOh, the test, ok.
09:06heycamnox: just following the convention of delegate+ instead of r+ for stylo-related changes, so that the patch author can be in control (kind of) of when it merges
09:06noxTIL there is such a convention.
09:07heycamjust a recent one
09:07heycamsince auto-merging of servo changes into mozilla-central can cause things to break
09:10Ms2gerSo does github have a dashboard with the PRs people have requested review from me on?
09:11Ms2gerAh, https://github.com/pulls/review-requested
09:12heycamthat seems handy
09:14heycamI am not good at github http://mcc.id.au/temp/github.png
09:16Ms2gerheycam, 6308/1575 here
09:16heycamnice
09:17Ms2ger155 repos below it
09:17heycamonly 128 for me
09:26wafflesMs2ger, I&#39;m fairly convinced that I can&#39;t keep up with Github
09:26wafflesjust crossed 2k
09:28Ms2gerIt&#39;s useless, of course, because I follow through email
09:28wafflesyep, same here :)
10:26noximperio: Don&#39;t be sorry, it can wait. :)
10:26imperionox: yes sir!
10:27noxMs2ger: Btw,
10:27noxMs2ger: what was your issue with filter-intermittents yesterday?
10:27noxMs2ger: The crappy output?
10:43xidornhmmm, I probably should do p=20 instead :/
10:44wafflesxidorn, ?
10:46xidornwaffles: nope, I just feel that I may not be able to wait for that long to land a stylo patch in both sides
10:46noxIt&#39;s currently under test.
10:46xidornnox: yep, I see that :)
10:56xidornoops...
11:18Ms2gernox, no, https://github.com/servo/servo/pull/15940
11:18crowbotPR #15940: Update some wpt expectations. - https://github.com/servo/servo/pull/15940
11:18noxTIL https://github.com/servo/servo/issues/15055, this looks so awful.
11:18crowbotIssue #15055: Support shorthand property &quot;all&quot; - https://github.com/servo/servo/issues/15055
11:18noxMs2ger: No?
11:18noxOh.
11:19Ms2gerAnyway, pto this afternoon, see you tomorrow
11:19noxMs2ger: Have fun.
11:19Ms2gerThx
11:21noxavadacatavra: We have bigger problems than Windows.
11:21noxavadacatavra: http://build.servo.org/builders/mac-rel-wpt2/builds/2896/steps/test/logs/stdio
11:22avadacatavranox: ruh roh
11:24noxavadacatavra: I think it&#39;s in rust-websocket that there is an issue.
11:24avadacatavranox: agreed
11:25noxhttps://irccloud.mozilla.com/pastebin/lVgIlBkj/
11:25noxavadacatavra: ^
11:25noxThe tests that fail.
11:25* avadacatavra goes to look at rust-websocket
11:28avadacatavra...we need to have a chat with websocket spec about their usage of sha1
11:33waffleshuh
11:33waffleslooks like it hasn&#39;t been deployed
11:34noxwaffles: What do you mean?
11:34wafflesnox, avadacatavra isn&#39;t a reviewer, bors says
11:34noxSssh.
11:34wafflesheh
11:34noxDidn&#39;t she make a saltfs PR though?
11:34avadacatavrawaffles: it&#39;s handled!
11:34avadacatavra:p
11:34wafflesit landed
11:35wafflesnox, I did btw :P
11:35noxwaffles: You saw nothing.
11:35noxNo-thing.
11:35waffleslol
12:32noxavadacatavra: http://build.servo.org/builders/windows-msvc-dev/builds/1313/steps/compile/logs/stdio
12:32noxProgress.
12:37avadacatavranox: hyperbump is a marathon
12:43waffleshey jdm! o/
12:43jdmhi folks!
12:43wafflesjdm, start writing TWiS :P
12:43jdmuh oh
12:47noxhttps://irccloud.mozilla.com/pastebin/5FkjKVS4/
12:47noxavadacatavra: ^
12:48avadacatavranox: these are different than the other set right
12:48noxavadacatavra: Subset.
12:48avadacatavragotcha
12:48cynicaldeviljdm: o/ hello again!
12:49avadacatavranox: can you link me to the error output for those
12:49avadacatavrajdm: welcome back. good luck.
12:50noxavadacatavra: http://build.servo.org/builders/linux-rel-wpt/builds/2882
12:54noxavadacatavra: So /websockets/Close-1000-reason.htm passes now,
12:54noxavadacatavra: but not /websockets/Secure-Close-1000-reason.htm.
12:54avadacatavraahhh
12:55avadacatavrawell that makes sense cause the hyper bump required changes to the secure websockets
12:56avadacatavralooks like it&#39;s all the secure stuff failing now so i&#39;ll look at those
12:57noxavadacatavra: They don&#39;t even open.
12:57noxavadacatavra: Don&#39;t forget to retrieve my hyper branch.
12:57noxavadacatavra: Meanwhile I&#39;m still trying to make windows green.
12:58noxjdm: We really should get rid of our usage of rust-websocket.
12:59noxavadacatavra: http://build.servo.org/builders/windows-msvc-dev/builds/1314/steps/compile/logs/stdio
12:59noxWOOT
12:59noxjack: ^
12:59noxjack: Green Windows build for Hyper bump.
12:59avadacatavranox: nice work
13:00noxstandups: Fixed the Hyper bump on Windows.
13:00standupsOk, submitted #43735 for https://www.standu.ps/user/nox/
13:08noxSimonSapin: http://build.servo.org/builders/windows-gnu-dev/builds/1309/steps/test/logs/stdio
13:08noxstandups: Hyper bump. :/
13:08standupsOk, submitted #43736 for https://www.standu.ps/user/nox/
13:08noxjack: ^
13:09noxSimonSapin: Fortunately, I filed https://git.io/vyD3W
13:09noxstandups: !remove 43736
13:09standupsnox: Huh? Try !help.
13:09noxstandups: !help
13:09noxstandups: !delete 43736
13:09standupsOk, status #43736 is no more!
13:09SimonSapinnox: maybe explain why in https://github.com/servo/saltfs/pull/620
13:09crowbotPR #620: Remove windows-gnu-dev builds - https://github.com/servo/saltfs/pull/620
13:29aisha_Hello Everyone, I am trying to make a website which works better on servo compare to another browser. So, for this, I am checking some website I was trying this website http://rgacademy.net.in/Demo-project/ but this isn&#39;t loaded proper but when I ran this Mozilla site https://www.mozilla.org/en-US/ load properly and not a single problem with this site can
13:29aisha_someone tell me. Why this site works properly on the servo?
13:30jdmaisha_: is the question why mozilla.org works in servo?
13:31aisha_jdm: yes, mozilla.org load properly but other site not loaded properly
13:56jdmaisha_: mozilla.org does not use any web APIs that aren&#39;t yet supported in Servo
13:57jdmaisha_: http://rgacademy.net.in/Demo-project/ looks like it loads fine in servo to me, too
13:57ajeffreyjdm: welcome back!
13:58ajeffreyDoes anyone recognize the CRASH in http://build.servo.org/builders/linux-rel-wpt/builds/2876/steps/test__1/logs/stdio?
13:58ajeffreyhttps://irccloud.mozilla.com/pastebin/FiYrnWsi/
14:00jdmajeffrey: it might be https://github.com/servo/servo/issues/12797
14:00crowbotIssue #12797: Intermittent timeout in /html/browsers/browsing-the-web/navigating-across-documents/014.html and /html/browsers/browsing-the-web/navigating-across-documents/013.html and /html/browsers/browsing-the-web/navigating-across-documents/015.html - https://github.com/servo/servo/issues/12797
14:00avadacatavrastandups: working on fixing the websocket tests for hyper bump
14:00standupsOk, submitted #43756 for https://www.standu.ps/user/avadacatavra/
14:01ajeffreyjdm: #12797 is about console logging being lost isn&#39;
14:01crowbotIssue #12797: Intermittent timeout in /html/browsers/browsing-the-web/navigating-across-documents/014.html and /html/browsers/browsing-the-web/navigating-across-documents/013.html and /html/browsers/browsing-the-web/navigating-across-documents/015.html - https://github.com/servo/servo/issues/12797
14:01ajeffreyt it? This one looks like it&#39;s a tinyfiledialogs issue.
14:01jdmajeffrey: it&#39;s not about it being lost, but it&#39;s about the result of alert/console.log being interleaved
14:01jdmin ways that make the test harness confused
14:01jdmand since this test definitely calls window.alert, I am suspicious because the output looks similar
14:02* jdm isn&#39;t sure what makes it classified as a crash
14:02ajeffreyjdm: ah, so the problem is that we&#39;re spitting out stuff on stdout / stderr and the wptrunner thinks it&#39;s a crash.
14:02jdmyeah
14:03ajeffreyjdm: should I just mark it as CRASH and file an issue?
14:03aisha_jdm: http://rgacademy.net.in/Demo-project/ the vertical-scroll of this website not work properly
14:03jdmajeffrey: want to fix #12797 in a separate commit and try again?
14:03crowbotIssue #12797: Intermittent timeout in /html/browsers/browsing-the-web/navigating-across-documents/014.html and /html/browsers/browsing-the-web/navigating-across-documents/013.html and /html/browsers/browsing-the-web/navigating-across-documents/015.html - https://github.com/servo/servo/issues/12797
14:04ajeffreyjdm: er, is there a plan of how to fix #12797?
14:04crowbotIssue #12797: Intermittent timeout in /html/browsers/browsing-the-web/navigating-across-documents/014.html and /html/browsers/browsing-the-web/navigating-across-documents/013.html and /html/browsers/browsing-the-web/navigating-across-documents/015.html - https://github.com/servo/servo/issues/12797
14:05jdmajeffrey: yes, make make console.log take the stdout lock as well
14:05jdmer, stderr
14:05noxajeffrey: You did exactly the same thing,
14:05noxbut with a match instead of an if let.
14:06ajeffreyjdm: can I do that later? #15536 has been sitting in the PR queue for over a month now.
14:06crowbotPR #15536: Implement setter for document.domain - https://github.com/servo/servo/pull/15536
14:06ajeffreynox: yes I did,
14:07ajeffreynox: your coding style is to put braces round nested match statments?
14:07noxYou know... Like rustfmt&#39;s coding style.
14:08jdmajeffrey: fine
14:08ajeffreynox: does rustfmt enforce this? (goes and finds out)
14:08jdmajeffrey: although I doubt that the test failure is consistent, so you probably don&#39;t want to change it to expected CRASH
14:09ajeffreywe certainly have lots of nested unbraced matches in the code at the moment.
14:09noxajeffrey: Don&#39;t know/care, but I&#39;m pretty sure it does,
14:09noxand I don&#39;t care we have lots of these,
14:09noxlet&#39;s not add new ones?
14:09ajeffreyhttps://irccloud.mozilla.com/pastebin/SEuO3bD2/
14:09noxI know.
14:10noxajeffrey: But your replacement of the code I suggested to write is still longer than mine.
14:10ajeffreynox: I&#39;m not sure we have an agreed coding style here.
14:10noxDo we need one for you to address my comment?
14:11ajeffreyjdm: hmm, true.
14:11ajeffreyaslo, difficult to test on my end because I do have tinyfiledialogs installed :/
14:14ajeffreynox: I prefer the unbraced version, you prefer the braced version,
14:14noxajeffrey: I just pasted code that shows my version is shorter, can we stop debating this?
14:16ajeffreynox: er, hang on we got into all of this because you didn&#39;t like the fact that one of the lines was pushing 120 characters.
14:16ajeffreydoesn&#39;t your version still have the long line?
14:17noxajeffrey: 1/ That was a different comment,
14:17noxajeffrey: 2/ that comment was &quot;Nit: you can merge the two arms above together AFAICT.&quot;,
14:17noxand I then provided a sample code showing what I meant,
14:17noxand assumed you would then wrap the lines accordingly on your own so that they are not extra long.
14:17noxMy last version of that snippet wraps it and you just have to copy and paste it.
14:20ajeffreynox: I give in, I&#39;ll just use your code.
14:21nox
14:22noxajeffrey: Btw Ms2ger and I started running rustfmt on some DOM files, so with time it will normalise itself.
14:23noxajeffrey: Did you lose a comment again on GH? :&#39;(
14:24noxajeffrey: https://github.com/servo/servo/pull/15536#discussion_r106173537
14:24crowbotPR #15536: Implement setter for document.domain - https://github.com/servo/servo/pull/15536
14:24noxThis was the last comment I mentioned.
14:24noxSee, no long lines, no nested matches, only cleanliness.
14:25ajeffreynox: You gave two versions, I picked the one I preferred.
14:26noxajeffrey: So why did you mention you rewrote my original code because of long lines, when you chose the long lines version..?
14:26noxOh well.
14:26ajeffreynox: you were the one who wanted the line shorter!
14:27noxIn a different comment.
14:27ajeffreyindeed.
14:27noxAnd because we should always prefer shorter lines, 120 is the ultimate limit and rustfmt will aim for smaller than that.
14:27noxSo don&#39;t say you rewrote it to nested matches because of me, when you could have just put the arm&#39;s body on its own line.
14:28ajeffreynox: anyway, we have something we&#39;re both happy with,
14:29noxI&#39;m not happy with that long line, but this is already too long of a discussion about this.
14:29ajeffreyI &quot;just&quot; have to work out why it&#39;s not passing wpt :/
14:29noxideal_width = 80; max_width = 120
14:31sebanHi, how to make highfive unassign an issue?
14:34ajeffreyjdm: is it OK if I just file an &quot;Intermittent CRASH in /html/browsers/the-window-object/security-window/window-security.html&quot; issue?
14:34jdmajeffrey: yes
14:34ajeffreyjdm: OK, ta.
14:35noxAin&#39;t that an important test for that PR though?
14:35noxThat&#39;s what tests the presence of security checks,
14:35noxwhich are introduced in that PR.
14:36noxjdm, ajeffrey: ^
14:37jdmI don&#39;t think that test uses document.domain
14:37noxjdm: Doesn&#39;t change what I said.
14:38noxjdm: First commit in the PR is &quot;Added some same-origin-domain checks.&quot;
14:41ajeffreynox: yes, it&#39;s quite frustrating that the test harness is reporting a false positive CRASH,
14:41ajeffreysince that could be masking a true positive.
14:42noxajeffrey: Also can you file a follow-up about fixing most failures from that test?
14:42noxajeffrey: You added the security checks quite arbitrarily to some methods and not the others.
14:43noxajeffrey, jdm: Oh.
14:43noxajeffrey, jdm: Is that intermittent 100% related to alert?
14:43noxBecause if it is,
14:43noxplease add a security check to alert(),
14:43noxwhich will make it throw SecurityError,
14:43noxnot write anything anywhere,
14:43noxand the intermittent disappear.
14:44jdmthat sounds like a nice solution
14:46noxAnd don&#39;t forget to close #15962 when it&#39;s done.
14:46crowbotIssue #15962: Intermittent CRASH in /html/browsers/the-window-object/security-window/window-security.html - https://github.com/servo/servo/issues/15962
14:46ajeffreynox, jdm: hmm, does mean having one outlier method that&#39;s doing a security check just to work round a bug with the test runner.
14:46noxajeffrey: That commit is an outlier altogether.
14:47nox&quot;Added some same-origin-domain checks.&quot;
14:47nox&quot;some&quot;
14:47noxWhy some?
14:47ajeffreynox: er no it&#39;s not, it&#39;s implementing the security check that the spec calls for.
14:47noxWhat?
14:48noxajeffrey: You sound like alert shouldn&#39;t have a security check.
14:48ajeffreyAFAIK every security check added by that PR is one which the spec asks for.
14:48* ajeffrey goes and checks
14:49ajeffreynox: I don&#39;t think it does, but I could be wrong.
14:49noxajeffrey: https://github.com/servo/servo/blob/master/tests/wpt/web-platform-tests/html/browsers/the-window-object/security-window/window-security.html#L120
14:50noxMaybe the test is wrong, but at least it seems to say it should throw.
14:50jdmnox: https://github.com/servo/servo/blob/master/tests/wpt/web-platform-tests/html/browsers/the-window-object/security-window/window-security.html#L187
14:50jdmer, nevermind
14:50noxjdm: You blinked at the wrong moment when scrolling.
14:50jdmyep
14:51ajeffreynox: https://html.spec.whatwg.org/#dom-alert
14:51ajeffreythe security check isn&#39;t in the alert() method
14:51ajeffreyit&#39;s in the windowproxy -> window binding,
14:51ajeffreywhich will be implemented by XOWs.
14:52canaltinovaSimonSapin: hey, I want to implement `:-moz-empty-except-children-with-localname` pseudo-class but it also takes a localname as a parameter. So I don&#39;t think its implementation is the same as other &quot;state&quot; pseudo-classes. Can it be done currently? Probably I need a binding function for it, right?
14:52noxajeffrey: Ok.
14:53SimonSapincanaltinova: yes, &quot;states&quot; are a single bit each so it sounds like this doesnt fit in that mechanism
14:54canaltinovaSimonSapin: ok, so can I handle the parameters in servo currently?
14:55SimonSapinI dont understand this question
14:55noxajeffrey: So is it ok now?
14:56noxajeffrey: Was that intermittent the last issue?
14:56canaltinovaSimonSapin: for example this pseudo class takes a parameter like this: `:-moz-empty-except-children-with-localname(param)` I need to handle the param here. Can I get this?
14:58SimonSapincanaltinova: in components/style/gecko/selector_parser.rs, impl<&#39;a> ::selectors::Parser for SelectorParser<&#39;a>, youll need to add a https://docs.rs/selectors/0.17.0/selectors/parser/trait.Parser.html#method.parse_non_ts_functional_pseudo_class method
14:59SimonSapincanaltinova: at the moment the method is not there because the default method (which always returns Err(())) is used
15:00canaltinovaSimonSapin: okay, thanks!
15:04noxjdm: Oh.
15:04noxjdm: That&#39;s why there was two traverse_preorder loops for the form owner.
15:05jdmnox: hooray for tests!
15:05noxjdm: https://github.com/servo/servo/pull/15283#discussion_r100906087
15:05crowbotPR #15283: Implement the form owner concept - https://github.com/servo/servo/pull/15283
15:05noxThis change is what makes the test pass I think.
15:05jdmsounds plausible to me
15:06noxjdm: is_in_same_home_subtree returns false otherwise.
15:06noxBecause it calls root_element, which relies on IS_IN_DOC to get to the root faster.
15:08ajeffreynox: yes, the other failing tests are known intermittents, I&#39;m just trying to work out what would be the easiest thing to do for this one with alert().
15:08noxajeffrey: Let&#39;s just squash the commits and r+ it in its current state.
15:09ajeffreyjdm: one possibility would be to add a setter for tidnyfd_forceConsole: https://github.com/jdm/tinyfiledialogs-rs/blob/master/libtinyfiledialogs/tinyfiledialogs.c#L118
15:09ajeffreysince this switches off the message that is causing the problem,
15:09jdmajeffrey: I don&#39;t think that avoids the problem here
15:10ajeffreythen run the CI with forceConsole=1.
15:10noxThe problem is a race between how console.log and alert print stuff, AFAIK.
15:10noxnagisa_hates_simd: The crate, or the whole concept?
15:10ajeffreynox: ah, it&#39;s not the warning message that&#39;s the problem,
15:10jdmright
15:10ajeffreyit&#39;s the actual text prompt?
15:11noxIt&#39;s the actual printing.
15:11noxCf. jdm&#39;s earlier link,
15:11nagisa_hates_simdthe instruction set, nox
15:11ajeffreyjdm: oh OK, hence locking.
15:11jdmthe indicator here is the lack of newline between the window.alert output and the console.log testharness output
15:11noxhttps://github.com/servo/servo/issues/12797#issuecomment-238865607
15:11crowbotIssue #12797: Intermittent timeout in /html/browsers/browsing-the-web/navigating-across-documents/014.html and /html/browsers/browsing-the-web/navigating-across-documents/013.html and /html/browsers/browsing-the-web/navigating-across-documents/015.html - https://github.com/servo/servo/issues/12797
15:11jdmand the extra newlines at the end of the output
15:12ajeffreynox: OK, I&#39;ll squash and r=you.
15:12ajeffreynox: thanks!
15:13noxjdm: Commented the change with the 2 loops,
15:13noxjdm: please r+.
15:13jdmnox: we&#39;ll need to merge the html5ever changes again
15:13noxjdm: Also I amended the damn WIP REMOVEME commit...
15:14noxjdm: But do I have your approval?
15:14noxjdm: I can just land everything.
15:14noxjdm: Oh ok I see what you mean, I didn&#39;t do the h5e PR yet.
15:14jdmnox: some kind of comment about why the separate loops are required would be good
15:15noxjdm: &quot;Commented the change with the 2 loops,&quot; :)
15:15jdmah
15:16globhow long do the servo tests take to run?
15:16glob(the github ones)
15:16noxglob: Do you mean locally, or in CI?
15:17globnox: CI
15:17noxglob: ~45 minutes per PR
15:17globta
15:17jdmnox: and yes, good to land when html5ever is ready
15:17noxjdm: Good.
15:18jdmthanks for running with that!
15:25jacknox: yay \p/
15:30jacknox: death is a large part of your commit messages
15:31noxjack: Well, I did kill treeherder once.
15:31noxSo not just the messages.
15:31noxjack: It&#39;s my Erlang background I guess.
15:32noxBrutally killing children like it&#39;s nothing.
15:45SimonSapinjack: ping
15:46noxjack: Wait, killing the build is happening NOW?
15:46noxCool.
15:46noxstandups: Reviewed and approved PR #15536.
15:46crowbotPR #15536: Implement setter for document.domain - https://github.com/servo/servo/pull/15536
15:46standupsOk, submitted #43759 for https://www.standu.ps/user/nox/
15:49cynicaldevilI&#39;ve got some questions about document.open(), would it be ok to just ask them here?
15:49noxcynicaldevil: Yes.
15:49cynicaldevilDoes the method open a new document? Or the existing one?
15:50noxcynicaldevil: So this is the weirdest method in the platform.
15:50noxcynicaldevil: New Window, same document.
15:50noxcynicaldevil: Turns out WebKit doesn&#39;t even create the new window, so I didn&#39;t either.
15:50cynicaldevilnox: That&#39;s not the first time you said that :P
15:51noxcynicaldevil: So we are following WebKit rather than the spec here, because the only reason we support it is because Google Docs needs it.
15:52ajeffreyjdm: hmm, the problem with tinyfiledialog alerts looks like it may be this: https://github.com/jdm/tinyfiledialogs-rs/blob/master/libtinyfiledialogs/tinyfiledialogs.c#L2427
15:52avadacatavraNox idk what I did with my computer. I will fix it and continue debugging tonight
15:52cynicaldevilnox: I don&#39;t understand the new window, same document part. Like, the same page is loaded in a new window?
15:52avadacatavraGo play over watch ;)
15:52ajeffreyit does a printf without a newline.
15:52noxcynicaldevil: Everything in the document is throw away,
15:53noxcynicaldevil: then the *same* document is transplanted into a new window.
15:53ajeffreywhich matches what we&#39;re seeing in http://build.servo.org/builders/linux-rel-wpt/builds/2876/steps/test__1/logs/stdio
15:53noxcynicaldevil: It&#39;s insane, as I said.
15:53cynicaldevilnox: ah ok
15:53noxcynicaldevil: https://html.spec.whatwg.org/multipage/webappapis.html#dom-document-open
15:53noxcynicaldevil: Steps 16-17.
15:54cynicaldevilnox: Also, we don&#39;t necessarily need to open the document before we write into it, right?
15:54noxcynicaldevil: Yep, you are right.
15:54noxcynicaldevil: You can write to a document while it&#39;s still parsing,
15:55noxcynicaldevil: and if it&#39;s done parsing, it will implicitly call document.open().
15:55cynicaldevilnox: oh
15:55jdmajeffrey: maybe, although I wonder what happens with _getch if that code is executed...
15:58ajeffreyjdm: blocks waiting on input?
15:59ajeffreyhmm, I really need to figure out how to fool tinyfiledialogs into thinking it has no graphics :/
15:59* ajeffrey tries sshing into localhost
16:04mbrubeckSimonSapin: Do we currently handle any Gecko-specific pseudo-classes, like :-moz-any (https://bugzilla.mozilla.org/show_bug.cgi?id=1340683)?
16:04firebotBug 1340683 NEW, mbrubeck@mozilla.com stylo: need to support -moz-any
16:04mbrubeckSimonSapin: If not, how do you want to handle this? Should I add a &quot;gecko&quot; feature to the `selectors` crate?
16:11jacknox: what do you mean? you made a PR. why not merge it?
16:11jacki haven&#39;t deployed it
16:11jackactually the reason is probably that it will break our nightly builds.
16:12jackso there may be some repair work to do after we deploy it to switch nightly builds to msvc if that wasn&#39;t done already
16:15ajeffreyjdm: how about if we just don&#39;t display tinyfiledialogs if we&#39;re running in headless mode?
16:16ajeffreyWoo hoo!
16:16jdmajeffrey: I&#39;m not sure what that means
16:17ajeffreyFinally! We have the setter for document.domain!
16:17SimonSapinmbrubeck: theres already a stylo-specific trait impl for parsing selectors https://github.com/servo/servo/blob/master/components/style/gecko/selector_parser.rs#L250
16:17ajeffreyjdm: at the moment, even if we&#39;re running in -z, we still pop up tinyfiledialogs.
16:17SimonSapinmbrubeck: the parse_non_ts_pseudo_class method is based on https://github.com/servo/servo/blob/master/components/style/gecko/non_ts_pseudo_class_list.rs
16:18jdmah
16:18mbrubeckSimonSapin: thanks
16:18bzImportError: No module named six.moves.urllib.parse
16:18bzWhazzat?
16:18SimonSapinmbrubeck: then for matching, the match_non_ts_pseudo_class method has a couple different strategies in https://github.com/servo/servo/blob/master/components/style/gecko/wrapper.rs
16:18bzAnd why is it busting all the stylo debug reftests?
16:19ajeffreyjdm: we should probably switch those off in headless mode.
16:19SimonSapinmbrubeck: like, some pseudo classes are &quot;states&quot; with a bit each in some bit field on element nodes
16:20ajeffreystandups: Landed PR #15536 &quot;Implement setter for document.domain&quot;.
16:20standupsOk, submitted #43761 for https://www.standu.ps/user/ajeffrey/
16:20crowbotPR #15536: Implement setter for document.domain - https://github.com/servo/servo/pull/15536
16:21mbrubeckSimonSapin: What does &quot;ts&quot; mean in these names, by the way?
16:21mbrubeckoh, tree-structural?
16:23SimonSapinyes
16:23jgrahamajeffrey: Terrifying :)
16:24SimonSapinmbrubeck: named after https://drafts.csswg.org/selectors/#structural-pseudos
16:25SimonSapinmbrubeck: but really, &quot;non_ts&quot; means &quot;anything that cant be implemented entirely in the selectors crate based on methods of the Element trait that are already needed to traverse the tree to implement combinators&quot;
16:25cbrewsterajeffrey: question about mutable vs immutable origins. Can we always use the host of the immutable origin for determining which event loop the pipeline should be in?
16:25ajeffreyjgraham: my vote for &quot;Most horrible web API.&quot;
16:26ajeffreycbrewster: yes, but with a couple of gotchas,
16:26ajeffreya) we have to use the eTLD+1, which isn&#39;t necessarily the one in the origin, and
16:27cbrewsterthats what reg_host handles right?
16:27ajeffreyb) we have to use a different mechanism for about:blank.
16:27jgrahamajeffrey: Yeah, it probably actually wins that one
16:27mbrubeckSimonSapin: `:-moz-any` would be simple to implement in the selectors crate, but it&#39;s non-standard. Should I still use the non_ts_* stuff, just as a hook?
16:27ajeffreycbrewster: indeed.
16:27ajeffreycbrewster: for about:blank my suggestion is to always load about:blank pages in the script thread,
16:28ajeffreythen notify the constellation,
16:28SimonSapinmbrubeck: I think Id rather keep non-standard stuff out of the selectors crate
16:28SimonSapinmbrubeck: another reason why the &quot;non_ts&quot; doesnt quite match reality
16:28cbrewsterajeffrey: hmmm, would that make it always synchronous?
16:28mbrubeckSimonSapin: specifically it&#39;s the non-standard predecessor to https://drafts.csswg.org/selectors-4/#matches
16:28ajeffreyrather than having the load initiated by the constellation and have it notify the script thread.
16:28SimonSapinright
16:29ajeffreycbrewster: if we need it to be async, we can always schedule a task to do it.
16:30ajeffreycbrewster: oh pox actually that may not work, hang on...
16:30mbrubeckSimonSapin: Hmm, this is the first &quot;functional&quot; pseudo-class in this list... we&#39;ll need to extend the enum definition to allow fields inside the variants...
16:30SimonSapin:-moz-any is also easier to implement than :matches, the latter has a specificity that depends on which element its matching
16:31ajeffrey