mozilla :: #fx-team

19 Apr 2017
11:40daopaolo: is there a bug filed on https://bugzilla.mozilla.org/show_bug.cgi?id=1351998#c7?
11:40firebotBug 1351998 FIXED, dao+bmo@mozilla.com Use CSS outline instead of border for XUL button focus ring
14:09mconleydao / florian: great job getting rid of the reflow on the tab strip when we're overflowed!
14:09* mconley adds to the list of reflows to write tests for
14:40Gijswhoa, they rolled back sumo
14:45spohlwhat's the best way to move a main thread i/o in a .jsm file to a different thread so we no longer block the main thread? should I be using workers?
14:45Gijsspohl: OS.File is the easiest, usually
14:47spohlGijs: I'm not familiar with OS.File, but does this look like something that could be converted to OS.File to you?
14:47spohlhttps://dxr.mozilla.org/mozilla-central/source/toolkit/modules/GMPInstallManager.jsm#423
14:47mythmonGijs: the Sumo rollback is planned to be temporary
14:49spohlGijs: and I would probably have to convert nsIZipReader's extract function to OS.File as well
14:49spohlhttps://dxr.mozilla.org/mozilla-central/source/toolkit/modules/GMPInstallManager.jsm#441
14:52Gijsspohl: ew
14:53Gijsspohl: uh, so in that case, I'm not sure.
14:53spohlGijs: ok, I think now that I'm reading this again, nsIZipReader's extract function must be implemented natively, not in JS.
14:53Gijsspohl: yes
14:53Gijsspohl: but it's also sync
14:53spohlso I guess I'll have to dispatch this to the gecko i/o thread
14:53GijsI don't know if its APIs are threadsafe
14:53spohlassuming we can make it async
14:53spohlyeah
14:53Gijsyou might have to create a separate API that doesn't suck
14:54Gijsif all you wanted to do was read content you could use XHR
14:54florianmconley: is it a known issue that the reflow counter in your add-on only seems to increase when doing stuff in the first browser window?
14:54spohlGijs: lol, yeah.. extractAsync, or asyncExtract
14:54mconleyflorian: hmmm
14:54mconleyflorian: sounds more like a bug in my Reflow API WebExtension thingy
14:54mconleyI'll file an issue
14:54Gijsspohl: but you want to write a file as well, and reading the entire thing into memory and then writing it out again would be dumb.
14:54spohlGijs: right
14:55Gijsspohl: though that would definitely be less effort... dunno how big the file you're writing there is. For small txt file or whatever it'd probably be fine, but if you're writing out 500k dlls, not so much
14:55mconleyflorian: https://github.com/mikeconley/reflow-api/issues/1
14:55Gijsoh geesh, it does sync looping over that thing as well
14:55Gijsugh
14:55florianmconley: thanks
14:55spohlGijs: hence why this came up as a quantum flow bug ;-)
14:56Gijsspohl: so yeah, you might be the lucky person to rewrite the nsIZip* things to not suck
14:56spohlGijs: yay
14:56Gijsspohl: I ran into this in an automated test before now, but that's just a test so perf is less important so I didn't scope-creep it to that point...
14:56Gijsbut yeah, those APIs are old and they suck
14:56Gijssorry
14:56Gijs:(
14:57spohlno worries. I kinda like this stuff actually, because it makes a measurable difference performance-wise
14:57spohlGijs: fyi, bug is here: https://bugzilla.mozilla.org/show_bug.cgi?id=1149732
14:58firebotBug 1149732 NEW, nobody@mozilla.org GMPInstallManager does a main thread IO
15:02Gijsmikedeboer: are you feeling better today? :)
15:03mikedeboerGijs: aye!
15:03mikedeboermuch
15:03Gijsmikedeboer: excellent
15:03Gijsmikedeboer: 2 more small reviews on the subview thing for the overflow panel (context menu fix alternative to changing the XUL popup manager (which turned out to be a Bad Idea), and a test)
15:03Gijsincoming
15:04Gijsspohl: I shall follow along and cheer you on :)
15:04spohlGijs: heh, you're too kind ;-)
15:05Gijsspohl: if you're in the market for this style bugs, maybe you could help Milan with bug 789945...
15:05firebothttps://bugzil.la/789945 NEW, nobody@mozilla.org Do automatic periodic async, off-mainthread writes for the prefs file when prefs change
15:05* Gijs has kind of abandoned that patch and it seemed (so far) like nobody had cycles to drive it over the finishline
15:09spohlGijs: wow, that is hairy on first read. does this have a perf impact that's significant enough to possibly nominate for quantum flow?
15:10spohlGijs: I'm asking because my focus is on qf and photon bugs at the moment, so it's unlikely I'll be able to pick up anything else anytime soon
15:15Gijsspohl: if we can eliminate the pref write on shutdown, I think so.
15:16Gijsspohl: it's hard to say whether that is feasible given current telemetry needs.
15:16spohlGijs: I see
15:16spohlI'll definitely read up on it, but no promises that I'll be able to take it on. :-)
15:17Gijsspohl: it's possible that a more expedient way of achieving the same goal is to allow append-only writes for changed stuff on shutdown, to avoid flushing the entire effin thing to disk while we're trying to finish closing Firefox
15:18Gijsspohl: basically, "hairy" is accurate, and this has the both-up-and-down-side of having a bunch of places that could do with improvement, and not a lot of clarity right now on what would be the quickest path to measurable "success" as far as qf is concerned
15:18Gijsie, we know the current state is awful
15:18Gijsthe question is what is the best/easiest way to get the 80% easy wins compared to that
15:20spohlGijs: makes sense
15:22Standard8florian: Im working on enabling the screenshots add-on in unit tests. Via try server, Im currently seeing: https://treeherder.mozilla.org/logviewer.html#?job_id=92429423&repo=try&lineNumber=2766
15:22Standard8any idea why those files suddenly show up as unreferenced on Windows?
15:22Standard8the langGroups.properites is whitelisted for mac
15:24florianStandard8: the first one is a devtools file, it shouldn't be reported when running in the bc suite
15:26florianStandard8: langGroups.properties is only packaged on Mac: http://searchfox.org/mozilla-central/source/browser/installer/package-manifest.in#699
15:26Standard8florian: huh, thats really weird
15:30* Standard8 downloads the windows zip build
15:35Standard8florian: err, this try build has it in omni.ja
15:35* Standard8 goes looks at a nightly build
15:35johannhdao: toolbarButtons.inc.css?
15:36johannhmaybe toolbar-buttons.inc.css
15:36daojohannh: too close to toolbarbuttons.inc.css
15:36daocan you rename this in a separate patch?
15:37johannhdao: egh, yeah, I was afraid I'd have to do that :P
15:37johannhsounds like a major mercurial pita
15:38johannhI'll do that then
15:38Standard8and not there on win32...
15:39johannhhmm maybe it works if I make a fresh patch based on central and rebase on that
15:45Gijsmak++
15:46GijsStandard8: that's a vm issue
15:46makkarma for restricting a bug comments? :)
15:46Gijsmak: yes.
15:46GijsStandard8: oh, that is, I thought it was... I've seen this on my trypushes and never see it on real trees
15:46GijsStandard8: that was on win7 vm runs though, and that looks like win8?
15:47GijsStandard8: do those jobs run (non-hidden) on the real trees?
15:47GijsStandard8: also, that's an artifact build
15:47GijsStandard8: it's totally possible this breaks for artifact builds but not for 'real' builds
15:47Gijs(and yes, we should fix that in the test if so)
15:48johannhdao: regarding the the forward button height, do we want the urlbar to have the same height on all platforms in compact mode?
15:48GijsStandard8: unless you're actually changing anything wrt that file I don't think you should worry about it (though filing a bug and asking florian about fixing it might make sense)
15:48johannhit's smaller for me on osx
15:49johannhmaybe that's something I broke, though
15:49daoshould probably be consistent
15:50Standard8Gijs: how did you work out its an artifact build?
15:50GijsStandard8: it's in the trypush syntax?
15:50daojohannh: it may depend on the font size
15:50daojohannh: there's also http://searchfox.org/mozilla-central/source/browser/themes/windows/browser.css#1154-1159
15:50GijsStandard8: ie I went to https://treeherder.mozilla.org/#/jobs?repo=try&revision=d49a34bf9b82c8719f2648e542df9e9754e87594&selectedJob=92429423 and hovered over the top commit
15:50johannhah, right
15:51Standard8Gijs: yeah. weird, I guess it is deciding to do artifact builds by default?
15:51GijsStandard8: what is "it" ?
15:51Standard8I dont think I request aritfact
15:52Gijshow did you push to try?
15:52Standard8via ./mach
15:52johannhso, it's larger on windows, that should mean that it sizes naturally on osx
15:52Standard8definitely didnt have artifact on there
15:52johannhI'll try to find out why it doesn't
15:54Standard8Gijs: oh well, Ill file a bug for now and ignore it
16:01Gijsmikedeboer: meeting ping? :)
16:02mikedeboerGijs: still got a minute :)
16:10Gijsmikedeboer: fwiw, https://bugzilla.mozilla.org/show_bug.cgi?id=1354071#c6 might be worth checking - otherwise I might need to add another patch...
16:10firebotBug 1354071 ASSIGNED, gijskruitbosch+bugs@gmail.com Switch overflow panel to using a panelmultiview
16:13mikedeboerGijs: you mean anchor styling in the new #appMenu ?
16:13mikedeboerGijs: oh, in the overflow panel
16:14Gijsmikedeboer: right. I haven't added anything that has subviews to #appMenu yet
16:14Gijsmikedeboer: we are somewhat in each other's "vaarwater" here, but not too much :)
16:14mikedeboermyeah, I'm thinking about that atm - will show code later today or tomorrow morning so we can discuss it. I agree it's ok to leave it outside of this bug
16:14Gijsmikedeboer: yeah, I'm out tonight, so discussing tomorrow sounds good to me
16:15mikedeboerGijs: okidoke
16:49sfosterjaws: I guess the first question on the downloading animation is whether we still have the download icon as a separate element in the toolbar, or if that's just a state for the library button.
16:49jawssfoster: the animation makes it look like a separate button on the toolbar
16:49sfosterit does, but is it clickable while its there?
16:54sfosterlooks like its unchanged in that it'll show the door hanger with download progress. Its just the animation indicating its available in the library that is new
17:17Asaanyone else noticing that painting seems to be slow in today's nightly windows build? I scroll and get blank spots that take a second or more to fill in.
17:18MossopI have noticed that
17:18MossopSomething seems to be lagging
17:22Asahttps://bugzilla.mozilla.org/show_bug.cgi?id=1357823
17:23firebotBug 1357823 NEW, nobody@mozilla.org painting slow in latest nightly
17:23MossopI just updated to the latest nightly and now Firefox won't start at all :(
17:25Asa:(
17:28sfostermconley: er, ahem. That last try push I botched a rebase and had reverted all my browser.js changes. So all those results are invalid.
17:29sfosters/rebase/merge/
17:43mattwmak: ping
17:55MossopApparently I needed to restart my machine to get Nightly to work again
18:40_6a68mconley: Hey, we're looking at adding a checkbox to one of the preferences screens to let users easily disable Screenshots. Right now, that requires manually editing xul files, correct? Is there any way we could dynamically add a checkbox from a system addon? I'm guessing not, because l10n?
18:41_6a68Screenshots is an embedded webextension, so there's a bootstrap.js that can call into services, etc
18:47mconley|livehacking_6a68: hey - livestreaming right now. Mind if I try to answer your question on air?
18:47_6a68sure!
18:47nalexandermconley|livehacking: ooh, awesome sauce.
18:47* _6a68 tunes in
18:47mconley|livehacking_6a68: oh, I mean, like, I was going to chat with you rhere
18:47mconley|livehackinger
18:47mconley|livehackinghere in IRC
18:47_6a68haha sure
18:48mconley|livehackingbut like, have my IRC client be visible in the stream
18:48mconley|livehackingokay
18:48* mconley|livehacking reads question again
18:48_6a68
18:48mconley|livehacking_6a68: What release are you targeting? 55?
18:49mconley|livehackingMattN: ping
18:49MattNmconley|livehacking: pong
18:49mconley|livehackingMattN: hey - I'm working on bug 1352501 right now, with clearance from canuckistani to get rid of the Reader Mode feature promotion panel when visiting an article for the first time. Do you know if we have any tours deployed that attempt to open this thing?
18:50firebothttps://bugzil.la/1352501 ASSIGNED, mconley@mozilla.com Remove Reader Mode feature promotion panel
18:50mconley|livehackingMattN: because I notice that there's UITour support for it as well
18:50mconley|livehackingand wanted to know if we needed to keep it for that too
18:50_6a68mconley|livehacking: sorry! 55 works, yeah
18:51mconley|livehacking_6a68: okay, in that case, l10n _shouldn_'t be a problem, as far as I know. There is an API to programmatically add XUL overlays for things
18:51MattN_6a68: The form autofill system extension is doing what you want
18:51_6a68I'm also wondering if we can expose a nice API for system addons to make changes like this post-57, to make it a little more modular
18:52mconley|livehacking_6a68: oh - perhaps follow MattN's example then - he's probably thought about this more than I have. :)
18:52_6a68mconley|livehacking | MattN: sweet!
18:52MattNwe're not using XUL overlays, just dynamically adding the content
18:52_6a68MattN: do you have a patch I could look at, by chance?
18:52MattNexample coming up
18:52_6a68
18:52MattNit's in m-c
18:53MattN_6a68: https://dxr.mozilla.org/mozilla-central/search?q=FormAutofillPreferences&redirect=false
18:53MattNbasically listen for "advanced-pane-loaded" then inject your content
18:53MattNhttps://dxr.mozilla.org/mozilla-central/rev/c0ea5ed7f91a6be996a4a3c5ab25e2cdf6b4377e/browser/extensions/formautofill/FormAutofillParent.jsm#96-102
18:54_6a68MattN: thanks, this looks perfect
18:54MattNmconley|livehacking: btw. there is another bug about the UITour style flush
18:54MattNtrying to avoid it
18:54MattNhttps://bugzilla.mozilla.org/show_bug.cgi?id=1352518
18:54firebotBug 1352518 NEW, nobody@mozilla.org Avoid calling getComputedStyle() in isElementVisible() in UITour.jsm
18:55mconley|livehackingMattN: yeah, I think I've seen that one. In this case though, I think we're able to remove the feature promotion entirely, at least on first article visit
18:55mconley|livehackingMattN: but I wanted to know if you know if there's still a use case for it being available for the tour
18:55mconley|livehackinglike
18:55mconley|livehackingMattN: right now, when I go to Help > Firefox Tour in release, I get sent to a SUMO page that doesn't seem to do UITour-y things
18:55mconley|livehackingdo we have an active tour anywhere?
18:56MattNmconley|livehacking: Just like 1352518, I'm not sure why this would be a P1 given it happens so rarely but okay
18:56mconley|livehackingMattN: happens during first use of the browser as soon as you visit an article. :/
18:57MattNright, but never again
18:57MattNanyways, a current problem that I was thinking about the other day is that there is no centralized way to know about tours going on
18:57MattNsince I don't always here about them I was considering adding telemetry for tours so I know which ones to QA
18:58MattNI don't think any tour involve and reader mode tour APIs at the moment though
18:58MattNI normally double-check with agibson though and get https://bedrock.readthedocs.io/en/latest/uitour.html updated
19:01MattNI would leave the targets in UITour though
19:03mconley|livehackingMattN: okay, thanks!
19:03MattNthough I guess that means UITour.jsm doesn't need to change
19:03mconley|livehackingcorrect
20:32bsmedbergAnyone here who knows about session restore?
20:32bsmedbergI have a bug and I'm not sure if there any logs or diagnostics I can provide when I file it.
22:01Mossopjaws: Is it known that unchecking "Use Master Password" does nothing in the options?
22:03jawsMossop: unknown, can you please file a bug? most likely broke with the re-org (bug 1335907)
22:03firebothttps://bugzil.la/1335907 FIXED, herrickz@msu.edu Re-arrange sections in about:preferences according to updated spec
22:19jawsthanks
20 Apr 2017
No messages
   
Last message: 40 days and 3 hours ago