mozilla :: #fx-team

17 May 2017
09:51johannhI can't click the menu button on central anymore
09:51johannhGijs: do you happen to know why? ^ :)
09:52johannhworks on Nightly
09:52* Gijs hasn't landed anything recently
09:52Gijserrors in the browser console?
09:52Gijsis the photon pref flipped or not?
09:52Gijstried a clean profile?
09:52johannhpocket button doesn't work either
09:52johannhclean profile
09:53johannhoff I think, one sec
09:53johannhdoesn't work with on either
09:54johannhcentral is in a pretty bad state in general
09:55GijsIf pocket doesn't work either, do any popups work, like the identity panel?
09:55Gijsalso, how are tests passing if all of this stuff is broken?
09:56Gijsare you sure this is a clean central build?
09:56Gijsa whole bunch of stuff just got backed out of central
09:56Gijs10 minutes ago
09:56Gijsjohannh: ^^
09:57johannhclean central on both OSX and windows
09:57johannhNS_ERROR_XPC_BAD_IID: Component returned failure code: 0x80570018 (NS_ERROR_XPC_BAD_IID) [nsIJSCID.getService]
09:57johannhidentity popup works
09:57johannhpage info window works, loading sites works, the zoom controls don't show the zoom level
10:00johannhaha backout fixed it for me on osx, but not windows, hmm
10:06Gijsrebuild harder on Windows?
10:06johannhyeah, I gave up, I probably didn't build it enough
10:07johannhGijs: on a related note, do you know who moved the new tab button to right on central?
10:07johannhor is that a regression, too?
10:08GijsI don't really know what you mean off-hand, so I guess that's a regression, too?
10:08* johannh sigh
10:08Gijsjohannh: is it just showing up at the end of the tabstrip or something?
10:08Gijseven when the tabstrip isn't full?
10:08Gijsor something else?
10:09GijsNightly from yesterday looks OK to me
10:09johannhGijs: ^
10:09Gijsjohannh: sounds like there's a JS error in the tabbrowser binding
10:15pastdaleharvey: is bug 1365275 going to improve the background color contrast between the toolbar and the location bar on Fedora?
10:15firebot NEW, Location and search bars become transparent on hover with a lightweight theme installed
10:15pastor is that by design?
10:16daleharveypast, yup that will fix it, upping a new patch shortly just waiting on a build to verify
10:16pastawesome, thanks
10:17daleharvey is by design
10:18pastyes, that is a fine contrast
12:34Gijsjohannh: did you figure out what was causing all the bugs you were seeing?
12:34johannhGijs: nope, not yet
13:08florianIs there an equivalent of the HTML <script ... defer> that works in XUL? browser.xul currently loads a big set of scripts that doesn&#39;t seem needed before the first paint (eg. nsContextMenu.js)
13:14makdao: did we change styling on the bookmarks toolbar? the dropdowns are too dark (they look black) on folders on Win10
13:14mak(the dropdown icons)
13:17daomak, yep:
13:17firebotBug 1361686 FIXED, Share bookmark toolbar button styling between platforms
13:17makdao: should I file a bug?
13:17johannhmak: dropdowns?
13:17makyes, the drop icon
13:17daomak: I guess so. not sure it&#39;s actually a bug.
13:17makdown icon, whatever you want to name that thing on the right of folders
13:17johannhmak: shorlander gave it to me as a temporary icon
13:18johannhmak: fun fact, Windows didn&#39;t have a dropdown icon on bookmark folders before that
13:18johannhso idk I think the color is intentional
13:18makit is different from any other glyph color on the ui...
13:19makincluded the chevron that is on the same exact toolbar
13:19makbtw, will screenshot, analyze it a bit and file a bug
13:32RyanVMjohannh: ping RE: bug 1337772
13:32firebot FIXED, Intermittent browser_context_menu_autocomplete_interaction.js | Autocomplete shouldn&#39;t appear - fals
13:33johannhRyanVM: pong
13:33RyanVMwas wondering if the last comment in the bug is still true
13:33johannhRyanVM: yeah, sorry, I got a bit distracted from that by Photon work, the intermittent this is causing still isn&#39;t fixed, so I&#39;m not sure what to do
13:34johannhRyanVM: unfortunately that intermittent is a beast, I spent a lot of time trying to figure out what exactly is wrong there
13:34johannhRyanVM: I&#39;m 99% sure that intermittent is not a user-facing problem and just a test bug
13:34johannh(or some other issue that was already there before that bug
13:35johannhso in theory we could uplift bug 1337772 without regressing anything, I think
13:35johannhbut I&#39;m not sure we have to do it
13:35makjohannh: dao: I don&#39;t know if from the sshot it&#39;s visible, I find a bit annoying that when I look up at the chrome the first thing I notice are these dropdowns. bug 1365593.
13:35firebot NEW, Bookmarks toolbar folders dropdown icons look darker than other UI glyphs
13:35RyanVMjohannh: I&#39;d say it&#39;s worth it only if the other orange it caused to spike wasn&#39;t worse than the original bug :)
13:36johannhRyanVM: well the bug is titled intermittent (and we fixed the intermittent), but along the way we also made the context menu <-> autocomplete behavior much better
13:37johannhOTOH I think it&#39;s fine to let this ride the trains
13:37RyanVMyeah, I&#39;m mainly asking because the failures still hit on Beta on TH :)
13:37RyanVMwe can wontfix 54 if the risk isn&#39;t worth it
13:38johannhcan you disable the test on 54?
13:38RyanVMi could, yes
13:39johannhI mean in the end you&#39;re the one who has to deal with the failures from bug 1260593 on beta, I&#39;m happy to uplift if you think that&#39;s bearable
13:39firebot ASSIGNED, Intermittent browser_context_menu.js | Test timed out - | Found a tab after previous test timed out:
13:39RyanVMyikes, that looks pretty frequent on trunk
13:40RyanVMyeah, let&#39;s just wontfix 54 and deal with it
13:40johannhI guess I should give that intermittent another look, it&#39;s just so time-consuming :/
13:43johannhmak: ok, hm, but isn&#39;t it the same color as the text?
13:43makjohannh: could be, those that matter?
13:43makI mean, it&#39;s a giant block of black
13:44makthe text is not filled
13:44makI can assure you in real life it&#39;s really really annoying :)
13:44johannhmaybe we can give them some opacity
13:44johannhthe photon spec says 0.6 opacity
13:44johannhmaybe we can just apply that
13:44makwindows explorer uses the same color as text, but the glyph size is half than ours
13:46makjohannh: I&#39;d also expect the chevron and arrows in menus to have the same color as these dropdown icons, for consistency
13:47* johannh regrets enabling this on windows :P
13:48johannhwe could just hide them again
13:48makwere them hidden before?
13:48makthat may explain why I noticed them so much :D
13:48johannhonly on OSX and Linux before
13:50makbtw, imho, either make them same color as the other glyphs, or half their size so they are consistent with the OS, or hide them.
13:52johannhpersonally I like the Photon glyph for those a lot better, but I think shorlander doesn&#39;t want me to change that until 57
14:16mconleybug 1291457
14:16firebot NEW, Use native arrow panel animations on macOS
14:31Gijsmikedeboer: besides tabindex attributes, you could also use CSS if that avoids cluttering up markup unnecessarily, I think
14:31Gijsmikedeboer: -moz-user-focus: normal, IIRC
14:31Gijsmikedeboer: but I haven&#39;t checked if we have obscure XUL code somewhere that is hardcoded to check for the attribute rather than checking the layout frame
14:31Gijs(which, you know, is possible...)
14:32Gijsmikedeboer: sorry for the delay, have been juggling a number of things :|
14:43mikedeboerGijs: gotcha! thanks for the CSS pointer
14:53* Gijs grumbles
14:54Gijsit would be so nice if mozreview&#39;s rebase stuff worked
14:54Gijsinterdiffs that cross rebases are unreadable
14:54Gijsespecially right now because they cross the task.jsm refactor
14:54Gijsand especially this patch because it was rebased across significant other patches to similar code
14:55* Gijs gives up and starts over
14:55jawsyeah they&#39;re pretty much garbage
14:56Gijson the flip side, imagine what our codebase would be like without code review
14:56globGijs: no.
14:57Gijsglob: ? Was that a &quot;no, I refuse to imagine such hell&quot; ?
14:57globGijs: correct
14:57Gijs(yes, I&#39;m sure I&#39;m supposed to use an emoji instead. no, I am not hip enough.)
15:16Gijsflorian: I&#39;m confused - are we still using yield and function* for add_task in tests?
15:16florianGijs: in which folder are you seeing this?
15:17GijsI just happened to look at that (it just landed)
15:17florianI&#39;m not planning on cleaning up devtools
15:17floriannor mobile
15:17Gijsoh, hm
15:17Gijsmobile especially seems like it&#39;d be worth it given the perf impact
15:17Gijsnot saying you personally should do it
15:18Gijsbut perhaps someone else wants to look at that?
15:18florianI&#39;ll be doing a second iteration targeted at these folders &#39;soon&#39;: &quot;accessible&quot;, &quot;addon-sdk&quot;, &quot;caps&quot;, &quot;docshell&quot;, &quot;dom&quot;, &quot;editor&quot;, &quot;extensions&quot;, &quot;gfx&quot;, &quot;image&quot;, &quot;js&quot;, &quot;layout&quot;, &quot;modules&quot;, &quot;netwerk&quot;, &quot;parser&quot;, &quot;security&quot;, &quot;services&quot;, &quot;storage&quot;, &quot;testing&quot;, &quot;uriloader&quot;, &quot;widget&quot;,
15:18Gijsis someone else looking at devtools at all?
15:18Gijsat least for non-test code?
15:18florianGijs: sure, someone should do it for devtools and mobile. I&#39;m just saying I&#39;m not volunteering to go fix all the test failures that will cause in these folders ;).
15:19florianI&#39;m happy to assist if someone wants help with running my scripts
15:20floriandevtools is a bit different, it has its own fork of Task.jsm called task.js
15:20Gijsoh dear
15:20Gijsis there a good overview of the perf benefits we saw on desktop after the removal?
15:21tromeythat fork was part of the de-chroming project
15:21florianI don&#39;t think the perf win was very significant. The talos numbers I looked at before landing were ~1%. It could be that we&#39;ll only see a real win once we&#39;ll remove the Promise.jsm imports
15:21tromeywe&#39;ve got a bug now to run the script to remove our task uses as well
15:22floriananyway, just the win in clarity of the stacks in the profiles made it worth it IMO.
15:22floriantromey: do you have the bug number?
15:23firebotBug 1365607 NEW, Switch devtools to async/await from Task.jsm/yield
15:24tromeyI&#39;m also curious about the perf win; the debugger at least is using some babel transform that strips out the async/await there and uses some babel runtime thing that&#39;s similar to Task
15:24tromey... and I was wondering how worthwhile it would be to drop that transpilation rule
15:58Gijsmikedeboer: hm, why the height additions of *all* the children, rather than just subtracting top/bottom as I suggested? :)
15:58Gijsmikedeboer: are there off-by-1 errors lurking somewhere due to rounding, or something?
15:59mikedeboerGijs: oh, that might work too... didn&#39;t realize your suggestion
16:00mikedeboerGijs: how is that different though? you mean the top of the first child, bottom of the last child?
16:00Gijsmikedeboer: yes
16:00Gijsmikedeboer: it&#39;s different because it&#39;s constant time rather than linear, for N children :)
16:01Gijsmikedeboer: plus every bounds check I think crosses the xpconnect/xpcom boundary, and this is perf-sensitive code, so seems worth optimizing if possible
16:06johannhseems like everyone loves the new dropdown arrows on windows
16:11Gijsanyone else recently switched to thunderbird and super annoyed about how it uses desktop notifications to notify you about email you received like 5 hours ago?
16:12Gijss/to thunderbird/to thunderbird 52/
16:27Gijsmconley: is there a sensible way of getting all the ohnoreflow-based bugs?
16:28mconleyGijs: they all have [ohnoreflow] in the whiteboard
16:28mconleyso whiteboard: [ohnoreflow] should do it
16:29Gijsmconley: thanks
17:03sfosterdao: I didnt find any docs for -moz-context-fill so I sketched in a page at - do you want to tech review?
17:03sfosterits linked from the CSS extensions page.
17:04daosfoster: looks good, but better ask jwatt
17:06johannhsfoster: example should maybe include the source of the SVG image, too :)
17:10sfosterjohannh: could do. Do folks use jsfiddle/codepen for the live examples? That might be best. Will have to read docs on the docs.
17:11sfosteryes, an example showing it actually working on a simple svg would improve it considerably.
17:12dao-moz-content-fill only works with chrome images
17:23daosfoster: ahem, it&#39;s -moz-context-properties
17:23sfosterdao: that isn&#39;t what I wrote? doh. thinking one thing and writing another.
17:24sfosterthat what you get for tring to do the right thing :)
17:26sfosterhrm, will probably need to delete and create new page. Editing it doesnt seem to have changed the url
17:30sfosterdao: did you have a good toolchain / process for producing all the icon svg files? They seem really clean.
17:31sfosteryou/whoever worked with you on them.
17:39bwintonsfoster: I hear they were artisinally hand-crafted by the UX team ;)
17:39bwinton(That&#39;s probably not true.)
17:40sfosterbwinton: actually it might be, if you look e.g.
17:40bwintonI did say &quot;probably&quot;
17:44shorlanderIt&#39;s mostly true ;)
17:45shorlanderWe are manually checking and running them through SVGO
17:47sfostershorlander: which svgo settings/plugins are you using?
17:48sfosterwe&#39;re getting exports from AE/bodymovin and turning those into svg. I can run that output through the same process.
17:59felipedid we just remove the borders around buttons on hover? felipe&#39;s law strikes again!
18:03shorlanderfelipe: have to keep it fresh ;)
18:03felipeindeed :)
18:07shorlanderAlthough I think the only difference from the default config is the indent spacing
18:08sfostershorlander: ok thanks
18:42_6a68gijs: hey, is there a clean way for a bootstrapped system addon to add a context menu item?
18:57_6a68Looking in dxr, it seems like direct XUL hacking is required
19:14rhelmer_6a68: yeah pretty sure we just insert the dom node into the right place for menu items, I don&#39;t recall an API for it.. gijs (or MattN maybe?) would know better than I
19:14rhelmeroh context menu you said... not as familiar with that actually, sorry I was thinking the top menu when I read that for some reason.
19:14_6a68rhelmer: no worries, looking in dxr, it seems like the same story
19:15_6a68do you know of any open bugs to add such an API?
19:15rhelmerwell, webextensions has an API ;) why not use that? it probably doesn&#39;t need to be on startup path right?
19:15_6a68we were discussing this a while ago along with menu items and toolbar buttons, but I don&#39;t recall that conversation leading anywhere
19:15_6a68ah, we&#39;re moving to a bootstrap-controlled button to start screenshots
19:16_6a68and it seems weird that the context menu item wouldn&#39;t be visible until you&#39;ve clicked the button at least once
19:16rhelmeroh so if it hasn&#39;t started yet.. ok.
19:16rhelmeryeah go it
19:16_6a68but then, we&#39;re talking about keyboard shortcuts as well, which I think probably requires some XBL too
19:16rhelmer_6a68: well DOM is an API :P
19:16rhelmerthe JS DOM API... sorry :P
19:17_6a68you&#39;re right though, we can at least contain the menu item management code to the extension itself
19:17rhelmeranyway yeah I think just modifying the xul dom is probably the best bet... I guess we could put in an API but I&#39;m not sure it&#39;ll really help tbqh
19:17_6a68modularity is the goal, not necessarily elegance or ease of use ;-)
19:18rhelmerwell I&#39;d rephrase - I don&#39;t know adding an API right now would be very useful, when we move the context menu away from XUL then we should reconsider
19:19rhelmeryou could get away from hardcoding IDs from the document in your add-on by always appending to the end or something
19:21rhelmer_6a68: you might find the impl for the webextension context menu helpful... it is in
19:21rhelmer_6a68: if you want to try to introduce a new internal API I&#39;d coordinate w/ webextensions folks, else just do it the way they do :)
19:22_6a68 thanks
19:22_6a68we&#39;re trying to minimize code running at startup, so it may turn out that it&#39;s best to just forget about the keyboard shortcut and context menu item until the button&#39;s been clicked
19:23rhelmerI think it&#39;d be fine from a startup perf pov, up to you though
19:23_6a68oh, good to know
19:23rhelmer(I saw that because for instance pocket does it)
19:26mixedpuppybgrins: ping
19:26bgrinsmixedpuppy: pong
19:27mixedpuppybgrins: Im fixing the webextension sidebar code to add menu items to the new dropdown
19:27mixedpuppyIm wondering what the plan is for the toolbarbutton
19:27mixedpuppywhether it is being removed, or kept
19:28bgrinsmixedpuppy: oh nice! I was planning to do that in bug 1360282 but happy to have help. The plan is to keep the toolbarbutton, but convert it to a plain toggle instead of opening up a panel
19:28firebot NEW, Remove the panel that opens when clicking on the sidebar toolbar button
19:29mixedpuppybgrins: cool, the patch I have removes adding webextension addons to the toolbarbutton, so that works for me
19:29mixedpuppybgrins Ill f? you on the patch
19:29mixedpuppyas there is a small item you may want to consider
19:29bgrinsexcellent! maybe we should file a new bug for your work, then I can do &#39;convert to a toggle&#39; in the existing bug
19:30bgrinsafter yours lands
19:30mixedpuppybgrins: bug 1365637
19:30firebot NEW, Sidebar doesn&#39;t show up in the new Photon sidebar selector
19:31mixedpuppybgrins: btw, is the toolbarbutton going to be a default?
19:32mixedpuppyif a webextension w/sidebar was installed, we moved the button into the toolbar (for the first extension w/sidebar, but not after)
19:32* mixedpuppy thinks hell leave that as is for now, it can be changed later
19:33bgrinsyes the toolbarbutton will be in the default set, i have on file for that
19:33firebotBug 1364238 NEW, Add the sidebar button into the default toolbar set
19:33bgrinswe might still want to keep that logic in case someone removed it from the toolbar, then adds a sidebar addon
19:33bgrinsi can double check in the structure meeting tomorrow
19:35mixedpuppybgrins: ok, if later you want to have WE stop moving the button into the toolbar file a bug
19:35mixedpuppyfor now Ill just fix the dropdown
20:37bgrinsmixedpuppy: just want to make sure we are on the same page for Are you planning to make that change or are you expecting me to?
20:37firebotBug 1365637 NEW, Sidebar doesn&#39;t show up in the new Photon sidebar selector
20:38mixedpuppybgrins: Im adding it
20:38bgrinsok great
20:53mixedpuppybgrins: you mentioned the location of sidebar css, but that file does not exist
20:54bgrinsmay need to pull m-c, it just landed yesterday
20:55bgrinsmixedpuppy: ah that was a typo - should be (not plural)
20:55mixedpuppybgrins: yeah, typo :)
18 May 2017
No messages
Last message: 7 days and 17 hours ago