| 06:05 | <MikeSmith> | nice to see progress being made on Upgrade Insecure Requests in gecko https://bugzilla.mozilla.org/show_bug.cgi?id=1139297 |
| 17:06 | <smaug____> | dglazkov: just random question. How do blink folks do code reviews? will some people end up doing lots of them or is the time to get a review occasionally too long |
| 17:07 | smaug____ | wonders how other open source projects (than Mozilla) do reviews |
| 17:19 | <jarek> | Hi |
| 17:20 | <jarek> | there seem to be two ways to get styling property or attribute value: |
| 17:20 | <jarek> | element.attributes.id.value vs element.getAttribute("id") |
| 17:20 | <jarek> | element.style.fontSize vs element.getPropertyValue("font-size") |
| 17:20 | <jarek> | are there any plans to deprecate the getter/setter API? |
| 17:21 | <jarek> | I mean element.style.getPropertyValue("font-size") |
| 17:26 | <jarek> | Also, how element.dataset.size is different from element.attributes["data-size"]? |
| 17:30 | <gsnedders> | deprecating it is kinda pointless, because practically they'll never get removed |
| 17:31 | <jarek> | gsnedders: by deprecating I mean discouraging their use |
| 17:32 | <jarek> | though I'm not really sure whether the getter/setter API is worse |
| 17:32 | <jarek> | what I don't like is the inconsistency |
| 17:57 | <dglazkov> | smaug____: there's definitely an occasional review lag. It's also a good idea to point the code review at someone who would be suitable for review (the git cl upload tool will suggest the reviewers). |
| 17:58 | <dglazkov> | smaug____: we have OWNERs that helps a bit with some parts of the code, and watchlists that will cc the potentially interested folks |
| 17:58 | <smaug____> | ok, OWNERs is like peers in Gecko |
| 17:58 | <smaug____> | (well, we have module owners and then peers) |
| 17:59 | <smaug____> | dglazkov: is it a problem that some people end up doing lots of reviewing? |
| 17:59 | <smaug____> | or do you have some tools to split reviews somehow evenly to potential reviewers |
| 17:59 | <jgraham> | smaug____: I have *no idea* why you might be asking that ;) |
| 18:00 | <dglazkov> | it's expected for a senior engineer to do more reviews |
| 18:00 | <smaug____> | :p |
| 18:00 | jgraham | understands that smaug____ spends about 26 hours a day reviewing |
| 18:00 | <smaug____> | right |
| 18:01 | <jgraham> | smaug____: FWIW the Opera/Critic solution is to assign the reviews automatically to people based on user-defined filters |
| 18:01 | <smaug____> | meaning what? |
| 18:03 | <jgraham> | smaug____: You add a filter for the bits of the code you are allowed to review (and a "watching" filter for bits of the code that you are interested in but can't sign off on). Then when patches come in all the people with filters matching parts of the review are assigned those parts. It's up to them to sort out who takes each review. |
| 18:03 | <jgraham> | This also allows reviews to be done collaboratively, where that makes sense |
| 18:04 | <smaug____> | so review request ends up to some generic queue |
| 18:05 | <jgraham> | if you like, but the point is that it becomes an organisational problem to distribute the reviews in the most efficient way rather than making the requestee pick a reviewer with no context |
| 18:06 | <jgraham> | c.f. the way that bz deletes the string "bz" from his bugzilla handle when he is overloaded or going to be away |
| 18:06 | <jgraham> | That's pretty clear evidence that "submitter picks a reviewer" doesn't work |
| 18:07 | <smaug____> | usually it works quite well |
| 18:07 | <smaug____> | I wonder whether the generic queue approach makes then certain people to do even more reviewing |
| 18:07 | <smaug____> | do you recall if that was an issue at Opera? |
| 18:08 | <jgraham> | Well, I recall that some people did a lot of reviewing, but I couldn't tell you if they considered it a problem of the approach, or something that was worse with that approach than other approaches |
| 18:09 | <jgraham> | I believe that the overwhelming majority of people liked the review tool overall |
| 18:09 | <jgraham> | so I guess that suggests they weren't totally overloaded |
| 18:19 | <smaug____> | (reviewing is usually fun, but I'm always interested in to improve the process) |
| 20:15 | <jamesr___> | SimonSapin: wtf8 is brilliant |
| 20:15 | <SimonSapin> | the idea is not new, but it was a bit hand-wavy before |
| 20:16 | <jamesr___> | yeah, but could be pretty useful in a browser engine that needs to support DOMString everywhere but doesn't want to have half null bytes |
| 20:17 | <jamesr___> | blink detects and uses a bit for 8 bit strings |
| 20:17 | <SimonSapin> | that was the original motivation indeed (in Servo) |
| 20:17 | <jamesr___> | i think some VMs do too |
| 20:18 | <jamesr___> | does ***monkey? |
| 20:18 | <SimonSapin> | yes |
| 20:18 | <jamesr___> | i think the dart vm uses either 1, 2, or 4 bytes per character for a string |
| 20:18 | <jamesr___> | and a particular string is one or the other |
| 20:18 | <jamesr___> | (or at least that was a plan, not sure if it's implemented) |
| 20:18 | <SimonSapin> | so does python 3.3+ |
| 20:18 | <jamesr___> | so indexing in a particular string is always constant time |
| 20:18 | <jamesr___> | but a big string with one surrogate pair is 3/4ths 0x00 |
| 20:19 | <SimonSapin> | in SpiderMonkey: https://blog.mozilla.org/javascript/2014/07/21/slimmer-and-faster-javascript-strings-in-firefox/ |
| 20:19 | <jamesr___> | what does servo use now? |
| 20:19 | <jamesr___> | ah yeah, looks about the same as blink |
| 20:19 | <jamesr___> | assuming there's a bit in the string header for which it is? |
| 20:19 | <SimonSapin> | I think Servo uses UTF-8 (Rust’s native string type), so unpaired surrogates in the DOM are replaced with U+FFFD |
| 20:20 | <SimonSapin> | it’s unclear if this is a real web compat issue |
| 20:20 | <jamesr___> | yeah that's going to be a problem |
| 20:20 | <jamesr___> | people store all sorts of crazy shit in storage apis through DOMStrings |
| 20:20 | <SimonSapin> | and if it is, we’re undecided between WTF-8 and ill-formed UTF-16 |
| 20:20 | <jamesr___> | i can't remember the details but we've broken several web properties by accidentally doing unicode conversions at different parts of our storage systems over the years |
| 20:21 | <SimonSapin> | but Rust ships with WTF-8 for OS strings on Windows in the standard library |
| 20:21 | <jamesr___> | where somebody assumed that a DOMString could be round-tripped to a unicode string (typically utf8) |
| 20:22 | <jamesr___> | filesystem APIs on non-windows have all sorts of crazy crap too |
| 20:29 | <hemanth> | can't let rest = await fetch(url); let data = await resp.json(); be reduced to a single statement ? |
| 20:49 | <TabAtkins> | jarek: For CSS, at least, the getter/setter pair mean you don't need to worry about the handful of randomly-renamed properties ("float" vs "cssFloat", due to ES3-era propery name restrictions). They're also required for any properties that don't propertly convert from snake-case to camelCase, like all custom properties. |
| 20:52 | <TabAtkins> | SimonSapin: Like jamesr___ said, using utf-8 for JS strings is *definitely* not web-compatible. A bunch of different tools rely on the fact that strings are effectively u16 arrays to encode arbitrary binary data into them. |
| 20:53 | <TabAtkins> | SimonSapin: Also, thanks for the Rust tree/parser/selector stuff! Will look it over eventually. |
| 20:54 | <SimonSapin> | for JS strings yes, agreed |
| 20:54 | <SimonSapin> | but for text in the DOM tree? |
| 20:55 | <jamesr___> | well depends on what you mean by 'in the DOM tree' |
| 20:55 | <jamesr___> | a lot of web APIs are specified in DOMString which is list of u16 |
| 20:55 | <jamesr___> | you'll need to hook those up to something in servo |
| 20:56 | <SimonSapin> | I mean the data of Text nodes, for example |
| 20:57 | <smaug____> | for Text nodes Gecko stores the strings using either 8 or 16 bits strings |
| 20:58 | <SimonSapin> | smaug____: the question is, is preserving unpaired surrogates there necessary for web compat? |
| 20:59 | <SimonSapin> | hsivonen was suggesting that maybe not |
| 20:59 | <jamesr___> | i would recommend testing that theory with telemetry data |
| 21:00 | <smaug____> | SimonSapin: I'd assume it is necessary |
| 21:00 | <SimonSapin> | yes, telemetry would be interesting |
| 21:00 | <jamesr___> | not all encodings web pages use are unicode too, remember |
| 21:01 | <SimonSapin> | jamesr___: do browser support encodings that don’t map to Unicode somehow? |
| 21:11 | <jamesr___> | that's a good question |
| 21:11 | <jamesr___> | even if browsers don't support it (i.e. don't render the text as expected) script in the page might expect data contents to round-trip |
| 21:40 | <gsnedders> | jamesr___: I remember some talk about flags for 8-bit strings being done on the leaves of the rope, so that a big string with one surrogate pair doesn't pay a huge price |
| 21:40 | <gsnedders> | jamesr___: idk if that ever happened though |
| 21:42 | <gsnedders> | TabAtkins: can we have style.float work nowadays? IIRC the spec says it should work now? |
| 21:44 | <TabAtkins> | gsnedders: We *can* have it work, yes; those naming restrictions were dropped in ES5. The spec does indeed say that el.style.float *should* be usable. |
| 21:44 | <TabAtkins> | Doesn't help custom properties, tho. |
| 21:44 | <gsnedders> | TabAtkins: I mean "may we have" :P |
| 21:44 | <gsnedders> | TabAtkins: I'm well aware ES5 dropped all this |
| 21:46 | <JoWie> | heh i added support for style.float in jsdom last week |
| 21:46 | <JoWie> | all browsers implement it now yes |
| 21:46 | <gsnedders> | yay! |
| 21:49 | <JoWie> | cssFloat is synced up with float |
| 21:51 | <JoWie> | just like the fontSize and ['font-size'] stuff |
| 21:51 | <gsnedders> | Yeah, indeed. |
| 22:03 | <gsnedders> | data:text/html,<script>document.write(location.href);</script> cross-browser returns a long string consisting of "data:text/html," repeatedly. how weird. |
| 22:04 | <JoWie> | that's because you're doing weird things :p |
| 22:09 | <jgraham> | gsnedders: hah. But also, really? That seems surpising because you would have thought that the url was set before the document was sent to the parser |
| 22:09 | <jgraham> | Oh |
| 22:09 | <espadrine> | so, according to standards, what should the number of data: thingies be? |
| 22:09 | <jgraham> | Yeah, no that's not surprising |
| 22:10 | <espadrine> | > An unbalanced tree was written using document.write() causing data from the network to be reparsed. |
| 22:11 | <gsnedders> | jgraham: ? |
| 22:19 | <SimonSapin> | gsnedders: I’m surprised that it’s not infinite |
| 22:19 | <SimonSapin> | there must be some kind of recursion limit to executing <script> from document.write |
| 22:20 | <gsnedders> | how's that the recursion? |
| 22:23 | <jgraham> | gsnedders: ? |
| 22:25 | <jgraham> | You load the page. It parses to after the </script> and then runs a script which document.writes the text "data:text/html,<script>document.write(location.href);</script>", which is parsed until after the </script>, which runs a script that document.writes "data:text/html,<script>document.write(location.href);</script>", which… |
| 22:26 | <gsnedders> | that was the guess I had… I didn't realise that caused a redirect… |
| 22:27 | <jgraham> | redirect? |
| 22:27 | <gsnedders> | ohhhhh |
| 22:27 | <gsnedders> | duh |
| 23:48 | <zewt> | http://stackoverflow.com/questions/12575572 so why exactly do people do this obfuscation where they pass a global into a function instead of just assigning to a local |
| 23:48 | <zewt> | i see it a lot and it always seems to do nothing but make code harder to follow |