mozilla :: #fx-team

14 Jul 2017
06:29gandalf!seen mconnor
06:29firebotmconnor was last seen 41 days and 11 hours ago, saying 'bhearsum: probably meant absearch. saw your email, will reply with more context and links there. ' in #releng.
09:05mikedeboerjohannh: belated pong?
09:07johannhmikedeboer: ah, cool, I'll have some questions in a few minutes
09:12johannhmikedeboer: hello!
09:13johannhI have two problems to figure out with the new panelmultiview thingy
09:14johannhwhat I'm doing right now is increasing the padding between the elements when a menu was opened with a touch gesture
09:14johannhwhich in itself is a few lines of CSS, but I've found that everything else in that patch is quite challenging
09:15johannhso now I have two problems with the panelmultiview
09:15johannhthe first is that it caches its previous height
09:16johannhwhich means that when you open the main menu with touch and then open it again with non-touch it stays really large even though the menu items are smaller sized
09:17johannhthe second is that I would have to increase the minimum width in touch mode
09:17johannhwhich also sounds like a line of CSS, but sometimes the width is dynamically set and I'm still figuring out the logic behind that
09:19mikedeboerjohannh: well, sounds like you want a method in PanelMultiView.jsm that looks and works like `forgetSizes` or something
09:19johannhyeah
09:19mikedeboer(or forgetCachesSizes)
09:19johannhI guess both are kind of the same problem
09:19mikedeboerbecause yeah, those are all optimizations
09:20johannhwhen is the width set dynamically via inline styles?
09:20mikedeboeron first panel open of the main view
09:20johannhlike, I can see the line of code but I'm not 100% sure why it's happening
09:20johannhah
09:21johannhmikedeboer: ok so currently I can only reproduce it like this:
09:22johannhopen the main (hamburger) menu
09:22mikedeboeryou know what, I think it's quite alright to reset the width caches when the panel is hidden
09:22johannh#appMenu-mainView does not have an inline style min-width
09:22mikedeboerso on the 'popuphidden' event
09:22mikedeboertrue
09:23johannhbut then open another subview and press the back button
09:23johannh#appMenu-mainView does have an inline style min-width
09:23mikedeboertrue - that's the mechanism that ensures that all the subsequent panels are of the same width as the main view
09:23johannh(I also found it a bit confusing that you call showSubView on popuphidden)
09:23johannhah, ok
09:24johannhright, that requirement makes sense
09:24johannhbut then yeah, I can reset it on hidden
09:24mikedeboerjohannh: that's making sure that the main view is set as the active view again for when the panel opens back up
09:24johannhmaybe it already is reset on hidden? I didn't check that, I assumed it needed to be persisted
09:25johannhmikedeboer: ah, maybe I'll add a comment in my patch ;)
09:25mikedeboerjohannh: that'd be nice! :)
09:25johannhok so the width problem sounds solved, then, thanks!
09:25johannhthe height, I'll have to figure out a criterion to invalidate it
09:26johannhmaybe I need to store if the panel was previously in touchmode or something
09:26johannhgetting a bit hacky
09:27johannhmikedeboer: maybe that's a question for Paolo, but what exactly does the height optimization solve?
09:29mikedeboerjohannh: I think you can use the same logic for that. It's there to make sure the calculated height is always correct, because the C++ panel impl has bugs
09:30johannhhuh, with same logic do you mean invalidate on hidden? doesn't that defeat the purpose?
09:30mikedeboerit doesn't calculate the height of xul:description and xul:labels that are multiline (with wrap=true)
09:30johannhoh
09:31mikedeboerso clearing the cache would actually make sense in your case
09:31mikedeboerit's all a big pile of menure to work around bugs elsewhere
09:31mikedeboer(exaggerating, the impl is actually quite nice overall)
09:31johannh:)
09:32mikedeboerwe'll be able to clean up the class one all panels use photonpanelmultiview
09:33johannhbut, sorry, just to clarify: are we talking about clearing the height cache on every popuphidden or just when leaving/entering touchmode?
09:35paolojohannh: which height cache are you talking about exactly?
09:36johannhpaolo: one second, finding the code in searchfox
09:37johannhpaolo: I _presume_ this line is causing my main menu to be too large https://searchfox.org/mozilla-central/source/browser/components/customizableui/PanelMultiView.jsm#470
09:38johannhhah, yeah, I can confirm it works fine when first opening the main menu via click and then via touch and then via click, but not if it first cached the touch size
09:38johannhso it caches the very first menu size
09:38johannhor view height
09:39paoloyeah, it doesn't sound right to cache that at all when we're showing a subview
09:39paoloi.e. it should likely be cleared when going back to the main view
09:40paolothat's required if we want to make this work for things like the Downloads Panel
09:40paolowhere the heioght of the main view changes
09:40paolowhile the panel is open
09:40johannhok, I think that works for my use case
09:41mikedeboerhowever, touch mode is not activated or deactivated when a panel is open
09:41mikedeboerso on popuphidden should be already fine. But you're going to separate that anyway
09:42paolomikedeboar: why is it only the main view to set the min height?
09:43paolomikedeboer: ^
09:44paoloi.e. with main view at 200px, subview at 300px, and sub-subview at 100px
09:44paolodoes the panel shrink to 200px when going from the subview to the sub-subview?
09:44paolowhy?
09:46johannhthat doesn't sound sooo bad to me, it's an edge case :)
09:47paoloright, just wondering why it's done this way
09:47johannhmight get more important once we start to do more with that panel
09:49mikedeboerpaolo: it's min-width
09:50paolowe're talking about min-height here I think...
09:50paolosee johann's link
09:54mikedeboerah ok, that I don't remember off-hand
09:55mikedeboerah bug 1365647
09:55firebothttps://bugzil.la/1365647 FIXED, mdeboer@mozilla.com Ensure that the height of panel views does not shrink smaller that of the main view
09:56mikedeboerwhich also makes sense, but just as eligible for clearing when johannh wants to
10:02paolook, sounds like the general idea could still be to never shrink the panel vertically when opening a new subview, although what we have now is a viable approximation
10:02paolothanks
10:07johannhso I'm now clearing on popuphidden and it works like a charm, thank you both!
10:07johannhpaolo: so, do you want me to clear on mainViewShown?
10:07johannhuh, showMainView9)
10:07* johannh types too fast
10:15paolojohannh: yeah, I think it makes sense
10:15johannhok!
10:16paolojohannh: I'm not even sure the member variable needs to exist on the object
10:16johannhlol https://searchfox.org/mozilla-central/source/browser/themes/shared/customizableui/panelUI.inc.css#9
10:17paolojohannh: well that was in 2014, might not be anymore ;-)
10:17johannhyeah someone should check that, maybe Gijs wants to give it a shot
10:19paoloI mean, there might be new code that sets the record that Gijs had to write more recently ;-)
10:19johannhooh
10:19johannhyeah, probably
10:19johannhif so someone should make a bug about updating that comment
10:20paolodefinitely! justa dding "the second ugliest", "third ugliest" if necessary would work
10:21johannhr- should be stored in a pref
10:21paoloah right, didn't think about this
10:22paoloshould be a preprocessor directive
10:22paolocontrolled by a pref
10:22johannhthat's true, that would actually work
10:22johannhuuh
11:10nhnt11is this tab crash at startup a known issue? :(
11:10* nhnt11 wonders if he missed something
11:11nhnt11yesterday a new profile worked fine, but today that isn't working either
11:12* nhnt11 wonders if it's an issue with artifact builds since Nightly works fine
12:26jawsdao: not sure if you saw this, but figured i'd share. starting with Windows 10 Creators Update, Microsoft removed the ability to change font sizes in Windows without installing a 3rd party application, http://www.makeuseof.com/tag/windows-10-fonts-text-sizes/
12:28jawsstill have to support win7 of course. this does make it harder to test different uses of firefox on windows 10
12:28daohadn't seen it yet. thanks
12:30jawsnp
12:39johannhpaolo: mikedeboer: another question for one of you
12:39johannhthe viewRect variable here is holding stale size information when switching to a subview https://searchfox.org/mozilla-central/source/browser/components/customizableui/PanelMultiView.jsm#601
12:39johannhbut I'm not 100% sure where it comes from and how to invalidate it
12:40mikedeboerjohannh: that's the rect of the view that's currently showing
12:40mikedeboer_before_ the transition starts
12:41mikedeboer(usually that of the main view)
12:41johannhhuh, but my main view has different dimensions
12:42johannhand this._mainView._lastKnownBoundingRect has the correct dimensions
12:47johannhmikedeboer: no, when I open the library view the debugger shows me that viewNode is also the library view
12:48johannhand it's then getting the wrong dimensions from the old library view
12:48johannhugh
12:48mikedeboerah you're right, it's previousRect
12:49johannhah, I'll just delete __lastKnownBoundingRect from all viewNodes on popuphiding
12:49johannhdoes that make sense?
12:49mikedeboerjohannh: yes.
12:49johannhcool
12:50mikedeboerdo you now know where `viewRect` comes from?
12:50johannhyeah that's viewNode.__lastKnownBoundingRect
12:51johannhor the product of some more calculation
12:51johannhhttps://dxr.mozilla.org/mozilla-central/source/browser/components/customizableui/PanelMultiView.jsm#683
12:51mikedeboeryup :)
12:54johannhyaaaay
12:54johannheverything works!
12:54johannhnow tests!
12:55johannhwhich might contradict my above statement ;)
15:35mconleyjaws: heyo - just in case your team ends up hitting any CART regressions, I just filed bug 1381037.
15:35firebothttps://bugzil.la/1381037 NEW, nobody@mozilla.org Remove the CART Talos test when Nightly becomes 57
15:35mconleysfoster: ^--
15:40jawsmconley: okay cool, will look at bug now to find out why lol
15:41mconleyshort answer: cart is not really measuring anything useful with Photon enabled.
15:41jawsyeah i see your comment, i'm not so sure it has no value at all though
15:41jawsit still does reflow the whole UI and reparent everything. it would be good to have some kind of measurement around to know when we have screwed it up
15:42mconleyjaws: I see. I don't _think_ it's capturing that though. CART and TART use the frame timing stuff to see how long it takes to paint frames on average. We might want to write a new test that tests how long it takes to go from starting customize mode to finally being in it
15:43mconleyI'll mention that in the bug.
15:43jawsokay yeah that would be cool
15:53TheOneOsmose: https://github.com/Osmose/mailman-admin-helper/pull/7
18:29bsmedbergmconley, I wonder how we could/would integrate the existing online docs with this new system
19:08mconleygood question
19:20gandalfflorian: I landed the update.locale I/O patch, but am wondering. Is there a difference between just using Fetch API and using requestIdleCallback? This kind of functionality could easily be delayed till idle, right?
20:04floriangandalf: these are 2 different problems: one is avoiding blocking main thread IO, the other one is scheduling things at a time when they won't get in the user's way. Yes, most code starting updates should wait for an idle callback instead of using raw timers.
20:06floriangandalf: btw, nsUpdateService.js still does (a lot of) main thread IO, bug 1361102
20:06firebothttps://bugzil.la/1361102 NEW, nobody@mozilla.org nsUpdateService.js does main thread IO during the first startup after an update, and whenever an upd
23:22adwshorlander: if you're still there on a friday, needinfo'ed you about turning the bookmark star blue in the page action panel in https://bugzilla.mozilla.org/show_bug.cgi?id=1381158
23:22firebotBug 1381158 NEW, nobody@mozilla.org Bookmark star icon in "Page actions" menu should be blue (not black) when filled in
15 Jul 2017
No messages
   
Last message: 71 days and 24 minutes ago