mozilla :: #fx-team

16 Mar 2017
00:00waglei have some brittle hacks that would be fine for months, but prolly not years
00:00waglelike flex seems broken
00:01wagleso use min-width
00:33wagleis there a current standard for adding persistance to the browser window state, or do I just make up one?
00:34wagle(xul's persist doesnt work for collapsed lists)
00:34Mossoppersist and localstorage is that standard so if it doesn't work then you're oout of luck
00:37waglehm.. will try harder then.. but how about things like this: elt = this._element(gPrefService.getCharPref("browser.bookmarks.editDialog.firstEditField"));
00:39wagleor is that deprecated?
00:39MossopI have no idea what that is
00:39wagleok, sorry
00:42wagleits a about:config type key/value store
00:45MossopOh I know what preferences are, I just don't know what this._element is or really what that line is doing so I can't comment on it
00:46waglei just meant that as an example of something in the browser .js using the about:config option/database.. but right now I'm reading up on local storage etc\
00:47MossopIt probably tends to happen for the cases where localstorage failed. It's not ideal
00:47wagleok.. that what I wanted to know
01:09wagleoh well, no write access?
01:10wagleno, that doesnt make sense, nm
01:11wagle(just the storage inspector)
02:08wagleeeeets alive!
06:23lenikHello all. Working on this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1346995
06:23firebotBug 1346995 ASSIGNED, lenikmutungi@gmail.com Replace promiseErrorPageLoaded with BrowserTestUtils.waitForErrorPage in browser_ssl_error_reports.j
06:24lenikShould I be using Firefox for Desktop Archifact Mode or Firefox for Desktop build?
06:25lenik*Artifact mode
07:27pastlenik: artifact mode will do just fine in this case
07:35lenikpast: unfortunately, I've already opted for Desktop Mode.
07:35lenikThanks though. Will remember it for future reference when working on these sorts of bugs
07:36pastno worries, both will work fine
07:36pastI have both locally and switch depending on the kind of work I need to do
08:47johannhlenik: you can still switch to artifact mode if you want faster compile times: https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Artifact_builds
08:47johannh:)
09:08lenikjohannh: hmmm...interesting. Will look at and see. I do tend to like being thorough, so I may stick to the Desktop Build for now.
09:09lenikUnless using the Desktop build means that the compiled code (don't know if that's the correct term) will be less platform-agnostic or something along those lines.
09:09johannhlenik: it's less about being thorough, it's more a question if you're touching C++ code
09:10johannhif you don't, you can safely compile with artifact mode
09:10johannhindependent of your platform
09:10johannh(and you're only changing JS for your patch, so you could use artifact mode)
09:10johannhthe advantage is that it's really _much_ faster
09:11johannha couple of minutes vs 30-50 minutes
09:11lenikOh... Well then I should switch then, since AFAIK I'm only using JS for this patch.
09:11lenikThanks for the clarification johannh
09:11johannhlenik: no problem :)
11:08ntimmikedeboer: ping
11:12mikedeboerntim: pong
11:13ntimmikedeboer: hey, I think I have a fix for bug 1347651, but I don't know how I can test it
11:13firebothttps://bugzil.la/1347651 NEW, nobody@mozilla.org Missing focus ring in findbar options buttons in OSX
11:13ntim(I'm on OSX)
11:13ntimbut the tab keys don't seem to cycle through the buttons
11:14mikedeboerntim: well, it does, try tabbing out of the textbox
11:14mikedeboerntim: you see that the arrow buttons do have a focus glow, but the others don't... you can hit the spacebar to activate a button
11:15mikedeboerntim: that way you see that the button is in fact selected
11:17ntimmikedeboer: tabbing out focuses the identity box for me
11:17mikedeboerntim: identity box?
11:19mikedeboerntim: I don't know what the identity box is...
11:19ntimmikedeboer: yes
11:19ntimmikedeboer: the box with the security info in it
11:19mikedeboerntim: we're both talking about the findbar?
11:19ntimmikedeboer: yes
11:19ntimmikedeboer: tabbing out focuses something in the URL bar
11:20mikedeboerntim: for me: open findbar, select the textbox, hit tab once and it focuses the 'previous' button (arrow pointing upwards0
11:21ntimmikedeboer: not the case on my side.
11:21mikedeboerntim: weird!
11:21mikedeboerbug!
11:21ntimmikedeboer: if I recall correctly there's a setting on OSX
11:21mikedeboeroooh yeah
11:21ntimI can't seem to remember which
11:21ntimor where to find it though
11:23mikedeboerntim: go to Mac > System Preferences > Keyboard > tab 'Shortcuts' > click radio button below on that page
11:23mikedeboerntim: simply do Fn + Ctrl + F7
11:23mikedeboer(shortcut)
11:23ntimmikedeboer: works, thanks
11:23mikedeboerI'm a keyboard monkey, so I have it enabled all the time
11:25mikedeboerntim: no, thank _you_ for jumping on it so crazy quickly!
11:25mikedeboer:)
11:42Aryxhi, for a testcase, i want to open N windows, each with M tabs. because this isn't a test, e.g. BrowserTestUtils is unavailable. what's a good way to achieve this?
11:48Standard8Aryx: is this via an add-on?
11:49Standard8If so, maybe WebExtensions have a way to do it
11:49Aryxno, i want to do it via the scratchpad, so chrome privileges (hunting a memory issue, try to find the conditions which trigger it)
11:49Standard8ah
11:50Standard8My only thought would be to copy some of the BrowserTestUtils code there but someone else might have a better idea
12:30mconleymikedeboer: heya - can I assume your r+ (once I fix the silly equality thing) still applies too my first patch in bug 1256472?
12:30firebothttps://bugzil.la/1256472 REOPENED, mconley@mozilla.com Try to reclaim session restore gains by restoring tabs lazily
12:30mikedeboermconley: absolutely. nice work.
12:30mconleyCool. Thanks for all of the reviewing and feedback!
12:30mikedeboermconley: btw, you're up early!
12:30mconleyMuch appreciated.
12:30mconleyTime change!
12:30mconleyor something
12:31mikedeboerok :)
12:31mconleythe daylight savings time adjustment has me all out of wack
12:36lenikjohannh: ping
12:37johannhlenik: pong
12:37lenikTrying to understand the function onLoad.
12:38lenikI don't know if that's overkill for fixing the bug.
12:39daomconley: fwiw, bug 1256472 looks like it will soon be irrelevant with bug 906076. your effort would probably have been better spent helping move the latter forward :/
12:39firebothttps://bugzil.la/906076 NEW, kevinhowjones@gmail.com Virtual tabs - lazily create linkedBrowser and other dependent elements for tabbrowser tabs to impro
12:40mconleydao: not 100% convinced of that yet, tbh. I suspect there are add-ons that attempt to access the browser after restoration that would cause one to be created.
12:40mconleyand maybe even browser code.
12:40mconleyless sure about the latter.
12:40lenikI don't understand how it works. I think I get what DOMContentLoaded does, and I was looking at this: https://www.w3.org/TR/DOM-Level-2-Events/events.html#Events-EventListener to understand the EventListener bits.
12:40daobrowser code is what we need to fix and where we can use help
12:41mikedeboerdao: plus it's nice simplification for the sessionstore code, so it's win in my view
12:41johannhlenik: it's not required to solve the bug but I can still help you understand it if you like
12:41mikedeboerI'm also kinda betting that our GSoC student should help out the lazy browser stuff
12:42daomikedeboer: I've only skimmed the patches, didn't look like simplification to me. but cool if it is
12:42mconleydao: I'm actually very curious to know the state of that bug - it's a little sprawling. Where are the current efforts, and where is help needed?
12:42johannhlenik: do you have a question?
12:42lenikAll the false, true stuff is confusing because I don't know what it is evaluating. at least I can tell that resolve is a lazily evaluated expression as per Promise.
12:43lenikjohannh: quite a few actually
12:43lenikWhat is DOMContentLoaded and how is the onLoad function working?
12:44daomconley: next steps are bug 1345098 and bug 1345090. they're driven by a contributor, so please coordinate with him if you want to help rather than just stealing bugs :)
12:44firebothttps://bugzil.la/1345098 NEW Lazy-browser-tabs: Deal with code which would unnecessarily/prematurely bind lazy-browsers
12:44firebothttps://bugzil.la/1345090 NEW Modify SessionStore to restore tabs with lazy-browsers
12:45lenikAnd is resolve *really* a lazily-evaluated expression? What's the false, true words for?
12:45johannhlenik: https://developer.mozilla.org/en-US/docs/Web/Events/DOMContentLoaded
12:46lenikThanks.
12:46johannhIt's a DOM event that basically signals that the page is done loading (although there's always a fine print)
12:47lenikjohannh: okay. So onLoad is a custom function of some sort?
12:49johannhlenik: which onLoad do you mean?
12:50zombiemconley: (or anyone) hey, you happen to know the estimate of how much data we send to child processes on creation, for things like prefs and initialProcessData and similar?
12:50johannh(Sorry had to get on mobile for a second, answers might be slower)
12:51lenikLine #988: browser.addEventListener("DOMContentLoaded", function onLoad()
12:51lenik^^That one. No worries
12:53johannhlenik: oh that's just the name that we give the function so the we can un-register it with the listener after we're done listening
12:55lenikjohannh: thanks. Got it
12:55johannhSo we need to replace that DOMContentLoaded event with BTU.waitForErrorPage, because in tests sometimes DOMContentLoaded although our content was not filled by our JS yet
12:56johannh*DOMContentLoaded fires
12:57lenikUnderstood
12:58shorlanderGijs_away, bwinton: Want to meet today?
13:10lenikjohannh: Here's what I understand of the snippet so far: https://pastebin.mozilla.org/8982206
13:11lenikI'd like to know where I'm wrong if that's alright
13:19Gijs_awayshorlander: would make sense to meet with jwatt and go over the perf numbers
13:21Gijsomg, they changed autoland to comment on bugs when autolanding fails
13:22Gijswho do I need to buy drinks for having fixed that?
13:24globGijs: mcote
13:24RyanVMmcote
13:24RyanVMheh
13:24globtwo drinks apparently
13:24Gijsheh, is that how it works
13:26shorlanderGijs: Ok, jwatt said he could join us
13:26shorlanderMy room in 5 minutes?
13:31Gijsshorlander: wfm
13:32shorlanderGijs: can you not hear me?
13:33Gijsjwatt: shorlander: https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=f53e35cb028f0e987c18387e12de8276b00cf955&newProject=try&newRevision=244ea3bad79b32693190246571dcaf02744eae0b&framework=1&showOnlyImportant=0
13:47Gijshttps://treeherder.mozilla.org/#/jobs?repo=try&revision=244ea3bad79b32693190246571dcaf02744eae0b&selectedJob=84135956
13:51jwattGijs, shorlander: https://wiki.mozilla.org/Buildbot/Talos/Tests#tpaint https://wiki.mozilla.org/Buildbot/Talos/Tests#ts_paint
14:03shorlanderGijs: https://bugzilla.mozilla.org/show_bug.cgi?id=1054016#c93
14:03firebotBug 1054016 NEW, nobody@mozilla.org Test performance impact of using SVG for Main Toolbar icons
14:15daoflo-retina: feel free to steal the review in bug 1347928
14:15firebothttps://bugzil.la/1347928 ASSIGNED, dao+bmo@mozilla.com Remove legacy information-*.png icons
14:17flo-retinadao: are you planning to do similar changes for the error-*.png and question-*.png files?
14:18daoflo-retina: yes, but it will probably be more effort since we don't have SVG equivalents yet
14:20flo-retinadao: I was considering filing a single bug for all of the unreferenced files in chrome://global/skin/icons/ from the list at https://docs.google.com/spreadsheets/d/1lthM18DHQUy2Spn1IN8w-nXHjxU9ion91p996GoAH94/edit#gid=0
14:22flo-retina(and I'm not going to steal that review, it looks good, but I'm not confident enough to r+ without testing the patch, and I don't have time to do that on each platforms now)
14:22daoflo-retina: some of these can probably be grouped in bugs, but yes... will be nice "good first bugs" too
14:23flo-retinadao: I'm hoping to land that test in the next few days, and I'll file grouped bugs for most of what remains, and maybe these can be turned into meta bugs if we need finer grained bugs to turn them into good first bugs
14:25johannhlenik: about the parameters (onLoad, false, true)
14:25johannhonLoad is the name of the function we want to un-register
14:25johannhand the other two arguments are documented here: https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener
14:26johannhit's a bit complicated so feel free to ask
14:36lenikjohannh: I think I've understood most except how exactly the true and false words work. What are they supposed to be doing?
14:38johannhlenik: they are parameters to the addEventListener and removeEventListener functions, they change how these functions work
14:55lenikjohannh: so the addEventListener and the removeEventListener functions are evaluated as boolean i.e. having a value of either true or false?
15:01johannhlenik: no, they accept booleans as options to configure their behavior
15:02johannhe.g. if the third value is true, it will use a capturing event listener (which is another difficult concept)
15:19sfosterdao: are there prefs or platform variations that drive manual/f11 full screen behavior. Because for me f11 fullscreens the entire browser viewport, no titlebar or toolbars visible.
15:20sfosterI'm on linux, will be in front of my mac later this morning.
15:23sfosterdao: so your thinking is that toolbars in this mode could have different backgrounds than otherwise, so we should calculate and cache their luminosity value separately? 'giving us a cache key like a stringified { id: toolbarid, active: true, fullscreen: false }?
15:36sfosterdao: would be good to jump on vidyo to quickly walk through this. I'm around for the next two hours
15:41* Mossop tries to find things he can do without a development machine
15:42Mossopmconley: Need more patches reviewed?
15:42mconleyMossop: oooh - let's see
15:42mconleythis kinda offer doesn't come around every day
15:43MossopSeriously folks, I review fast and my queue is generally empty because I'm not on a team as such
15:43mconleyhttps://bugzilla.mozilla.org/show_bug.cgi?id=1347069
15:43firebotBug 1347069 NEW, jjong@mozilla.com [DateTimeInput] (l10n) 12/24hr format for <input type=time> based on locale
15:43mconleyMossop: feel like looking at ^-- ?
15:43MossopSure
15:43mconleyMossop: I assume because you&#39;ve not got a development machine you can&#39;t investigate bug 1347514?
15:43firebothttps://bugzil.la/1347514 UNCONFIRMED, nobody@mozilla.org Customize tab doesn&#39;t open if it was replaced by a normal web page
15:44Mossopmconley: I&#39;m on my wife&#39;s laptop, doesn&#39;t even have xcode installed so I&#39;m pretty limited right now
15:45mconleyalright
15:45mconleyGijs / mikedeboer / jaws / MattN / johannh / dao: ^-- Mossop is looking for reviews to do
15:46jawsgood deal, thanks!
15:46mconleyMossop: maybe worth pinging smaug too, depending where the issues are.
15:48* Mossop suddenly regrets setting his bugzilla password to a 30 character random string that he has to copy off his phone
16:16johannhnoted!
16:25lenikjohannh: noted and thanks.
16:28lenikjohannh: btw, do I have to make sure that I thoroughly understand the code I&#39;m committing or my focus should be getting a general idea of what&#39;s happening and move on committing a patch etc.?
16:30lenikLike moving forward in the project itself, will I be expected to understand in general or [attempt to] understand thoroughly the code under review?
16:30johannhlenik: that&#39;s a good question. The purpose of a good first bug is usually to introduce you to the process of contributing and less to make you completely familiar with the whole code base
16:31lenikAh, so for now general idea works. Later on, thoroughness will be required.
16:31johannhHowever, you should definitely ask all the questions you need to understand at least what you&#39;re specifically changing and why
16:32lenikI hope the above is a concise summary
16:32lenik Will do.
16:34RyanVMmconley: I&#39;m not sure how to answer most of what you asked...
16:35mconleyRyanVM: hm... do you know who knows most about our testing infrastructure and how it runs in automation?
16:35RyanVMI&#39;m wondering if this may be an instance where it&#39;d be worth trying to remotely connect to a test machine while it&#39;s running tests
16:35RyanVMyou might start with ahal
16:35RyanVMhe&#39;s my usual go-to
16:35RyanVMotherwise, maybe even try downloading one of the CI builds and see if it reproduces locally?
16:36RyanVMannoyingly, we don&#39;t seem to take a screenshot under this failure scenario
16:36mconleyRyanVM: I tried the latter, and I&#39;m having issues with it complaining about the specialpowers add-on being not compatible
16:36RyanVMI wonder if there&#39;s a dialog up or something that&#39;d be telling
16:36mconleyalright, I&#39;ll talk to ahal
16:36RyanVMugh
16:37mconleyRyanVM: ahal is AFK. Can you think of anyone else?
16:38RyanVMjmaher maybe?
16:39mconleyalright, thanks
16:41ntimdao: we do have svg equivalents for question-*.png file
16:41ntimfiles*
16:42daontim: I don&#39;t think so. not sure
16:42daontim: oh, you weren&#39;t asking a question. okay then.
16:44ntimdao: https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/shared/in-content/help-glyph.svg
16:59lenikjohannh: quick question, what&#39;s the role of the object browser in &quot;browser.addEventListener&quot;?
17:20sfosterwhat is a good way in a mochitest to switch between active windows? I was using promiseFocus, but that&#39;s placing focus in the urlbar I think and I just want to toggle between 2 windows
17:21sfosteri.e. as if I alt-tab&#39;ed
17:32daosfoster: window.focus() and the activate event
17:35MossopI&#39;d use promiseWaitForFocus
18:05sfosterdao: ok thanks
18:06sfosterwill try that
19:00wagleI&#39;m looking for reviewers for https://reviewboard.mozilla.org/r/121128/ (prototype for resizing bookmrk editor window)
19:01Gijswagle: mak
19:01wagleyeah, he seemed to not want even more work, so..
19:02GijsMossop: ^^? :)
19:02* Gijs needs to run, unfortunately
19:02waglewhoops.. 8-D
20:32Mossopjeangong: I can&#39; make the meeting right now sorry
20:59manotejmekamattn: Hey how is it going? I have few questions regarding the subDialogue box searching. Jared and I discussed and we are implementing one of the solutions you have proposed. I am trying to implement the method where the buttons that correlate to popups has a new attribute/property that consists of its .dtd or .properties files.
21:00manotejmekaI am trying to use this https://dxr.mozilla.org/mozilla-central/rev/6d38ad302429c98115c354d643e81987ecec5d3c/browser/base/content/test/general/browser_misused_characters_in_strings.js#209 code to open up the file and read it to get the contents of it but I am getting a weird error that I can not debug. Was hoping you can help me out
21:03MattNmanotejmeka: what&#39;s the error?
21:07manotejmekahttps://www.irccloud.com/pastebin/WuVi5JQq/
21:07manotejmekamattn: This is the code I wrote to try making it work on my js file https://www.irccloud.com/pastebin/EayQJ6HE/
21:08manotejmekamattn: It errors on this line &quot;xhr.onreadystatechange = function() {&quot; and goes to xhr.send(null); and prints that message
21:10nalexandermanotejmeka: it&#39;s trying to parse the DTD as XML.
21:10MattNwhat nalexander said
21:10nalexandermanotejmeka: I think. You need to set the accepted content type, probably just to text/plain or similar.
21:10nalexanderMattN: is that how one does that with XHR?
21:10MattNI thought .response was for the parsed version, responseText was always text?
21:11MattNnalexander: I think either that or set .responseType
21:11johannh(can&#39;t you use fetch?)
21:11MattNjohannh: I don&#39;t think that will necessarily help here
21:11Mossopmconley: So in bug 1340842 you&#39;re going to see bimodal distribution since some tabs close with animation and some tabs close without. Maybe it would be better to have two different counters for those cases?
21:11firebothttps://bugzil.la/1340842 NEW, mconley@mozilla.com Instrument tab closing time from click to tab being gone
21:13manotejmekaMattN: So do I need to wait for it and have an yield or so?
21:13MattNmanotejmeka: you need to tell Firefox not to parse the response as XML
21:13johannhMattN: well it would be much more straightforward to use, don&#39;t you think?
21:14MattNmanotejmeka: set https://developer.mozilla.org/en-US/docs/Web/API/XMLHttpRequest/responseType to &quot;text&quot;
21:14mconleyMossop: oh, that&#39;s a great idea, yes.
21:14MattNjohannh: not really, the code he&#39;s referring to is using old styles though
21:14mconleythank you for reminding me about that
21:14MattNonreadystatechange shouldn&#39;t be used for that case
21:14Mossopmconley: I&#39;ll ignore that review for now then
21:14MattNmore verbose than necessary
21:14mconleyMossop: coolbeans
21:17manotejmekamattn: so I should do resolve(this.responseText); to be resolve(&quot;text&quot;);?
21:17MattNmanotejmeka: no, you need to set xhr.responseType = &quot;text&quot; before .open
21:19MattNI just tested that it works
21:21manotejmekamattn: Yes that works thank you
21:23manotejmekamattn: So it looks like rawContent is a Promise object which look like &quot; Promise { <state>: &quot;fulfilled&quot;, <value>: &quot;<!-- This Source Code Form is subject to the terms of the Mozilla Public\n - License, v. 2.0. If a copy of the MPL was not distributed with this\n....&quot; How can I access the <value> to parse that data?
21:23MattNmanotejmeka: It would be good to use `fetch` (https://developer.mozilla.org/en-US/docs/Web/API/GlobalFetch) for this (as johannh said) since it already returns a promise but I&#39;m not as familiar with it
21:23MattNI think you would use .text() on the response then
21:24MattNmanotejmeka: you can either yield on the Promise and assign to a variable or use .then((val) => {
21:24* MattN tries to find documentation
21:24johannhhttps://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch
21:24johannhis also pretty solid
21:25MattNlet val = yield openURL(&quot;chrome://browser/locale/preferences/preferences.dtd&quot;); is how you could use yield
21:26MattNbut you can&#39;t just yield in the scratchpad since you need to be in a generator function
21:27manotejmekaSo I should use fetch like this? https://www.irccloud.com/pastebin/A9R3zW2s/
21:27MattNanyone have recommend reading for promises with then+yield? Maybe http://2ality.com/2014/10/es6-promises-api.html
21:28MattNmanotejmeka: you want response.text(), not .blob() like I said earlier
21:28MattNyou don&#39;t need the second .then
21:29MattNthat&#39;s for displaying a blob as an image
21:29manotejmekaSorry I copied that from documentation and was asking. let me write the fetch and show you a step by step
21:29MattNmanotejmeka: I think it would be good for you to read http://2ality.com/2014/10/es6-promises-api.html or other documentation about promises in JS
21:29MattNcovering `yield` and .then()
21:30mconleyMossop: hey - have you met k88hudson before? She&#39;s working on Activity Stream, and has a question about a potential change to RemotePageManager.
21:30k88hudsonMossop: hey!
21:30Mossopk88hudson: Hi
21:30MattNmanotejmeka: apparently that page is outdated and replaced with http://exploringjs.com/es6/ch_promises.html
21:31MossopMattN: Is async/await appropriate here or do we still want to use Task.jsm for this sort of thing?
21:31MossopMaybe that&#39;s a more global question
21:31Mossopk88hudson: How can I help?
21:31MattNMossop: not sure, I guess it depends how this code will be used
21:32MattNThough I think we may want to have a discussion about async/await as a global thing
21:33* MattN doesn&#39;t know about the tradeoffs
21:33k88hudsonMossop: I was just wondering if there was a reason ports in the RemotePageManager don&#39;t expose their ids
21:35Mossopk88hudson: Looks like it is just the portID property on the port isn&#39;t it?
21:35MossopMan that code is ripe for switching to ES6 classes
21:35daoMattN: I think it came up somewhere in a newsgroup thread and the general expectation was that we&#39;ll migrate to the standardized stuff at some point. I don&#39;t think I&#39;ve seen any counter arguments
21:36k88hudsonMossop: yeah, but it doesn&#39;t get exposed in the publicMessagePort wrapper
21:36k88hudsonMossop: i was writing some code to do my own book-keeping of ports but then I could just use portID if it was exposed
21:37MossopOh right
21:37MattNdao: ok, that sounds familiar, if it totally replaces parts of Task.jsm then it may be good to start removing the old patterns
21:37* Mossop vaguely remembers this code ;)
21:39Mossopk88hudson: I guess I just tend to lean towards keeping things private unless there is a clear use case which makes maintaining the thing easier and safer in the future
21:41manotejmekamattn: Thanks for the help I will take a look at it and try to figure it out.
21:41MattNyou&#39;re welcome
21:42Mossopk88hudson: I wouldn&#39;t see an issue with making portID a getter on the public interface
21:46k88hudsonMossop: cool! if I did a patch could I ask you to review it?
21:46Mossopk88hudson: Sure
21:49Mossopflo-retina: How difficult would it be to script a change from Task.async/yield to async/await do you think?
22:01flo-retinaMossop: I was starting to think about it a few hours ago :)
22:04flo-retinaMossop: I would expect it to be relatively easy to do a script handling properly lots of cases, but we may need to cleanup by hand, and/or handle lots of cases by hand.
22:05flo-retinaMossop: is there an obvious way to identify a yield that&#39;s used to wait for a task, vs a yield in a generator actually used to generate values?
22:06MossopWell if the function is wrapped in Task.async then that&#39;s easy. If not then I think no there isn&#39;t an easy way.
22:06flo-retinalooking at http://searchfox.org/mozilla-central/source/devtools/client/inspector/fonts/test/head.js#20
22:07flo-retinait should be easy to handle openFontInspectorForURL, but selectNode is less obvious
22:08MossopHmm yeah that is a tough case
22:08flo-retinawe may be able to detect that it&#39;s called with yield/await somewhere else in the file
22:08flo-retinaso a script handling only when there&#39;s a Task.async wrapping should be straight forward
22:08flo-retinaa smarter (and significantly more useful) script would be a lot more effort
22:09* Mossop nods
22:09flo-retinahow often are we using actual generators in our code?
22:09flo-retinawould it be possible to identify all these cases and undo the action of the script on them?
22:10MossopProbably far less than we&#39;re using them as async functions. I can think of very few case I&#39;ve written myself
22:10flo-retinaa script could also handle all the add_task(function*() { ...
22:11flo-retinahow likely would we be to see the difference between a generator and an async function when doing a human review of the script&#39;s output?
22:11flo-retinahow helpful would it be to just push to try and debug until it is green?
22:12MossopDepends which way you do it. I&#39;d be nervous if you were switching all generators to async and attempting to use tests to spot those that weren&#39;t
22:13flo-retinatests + human code review
22:13flo-retinaand land at the beginning of a cycle
22:13flo-retinaI think the way I would go about it is to first script the replacement for the cases where we are 100% confident
22:14flo-retinathen make the script smarter to handle more cases
22:15flo-retinaand finally do a different patch that replaces all the generators to async, and carefully review that one. The goal of the previous steps would be to make that latter patch as small as possible, so that reviewers would be less likely to just scroll through a huge diff without seeing anything.
22:15* Mossop nods
22:15Mossopmakes sense
22:16flo-retinasounds like something to try next time I&#39;m bored while traveling then ;)
22:17MossopI&#39;m going to post to the newsgroups to see if anyone knows of a reason we shouldn&#39;t switch
22:18flo-retinaheh, I also wanted to ask if at this point we are sure it&#39;s not a change we may have to revert later
22:33flo-retinathe script should also remove Task.jsm imports
22:35MossopYou can probably ignore generator functions in scripts that don&#39;t import Task.jsm
22:39flo-retinafiguring out if Task.jsm has already been imported in the global scope or not may be difficult
23:15MattNMossop: Do you know if complete themes can add &quot;style overlays&quot;: https://developer.mozilla.org/en-US/docs/Chrome_Registration#style
23:16* MattN has never made a complete theme or used a style overlay
23:16MattNhttps://developer.mozilla.org/en-US/docs/Theme_Packaging doesn&#39;t mention chrome manifests at all
23:16MattNooh, https://developer.mozilla.org/en-US/docs/Building_a_Theme does
23:17MattNare there limitations on what complete themes can put in their chrome.manifest?
23:17MossopMattN: Looks like they can use style overlays
23:17MossopYes
23:18MattNok, thanks, email to firefox-dev coming soon
17 Mar 2017
No messages
   
Last message: 44 days and 5 minutes ago