1114532 - [Statusbar] Truncated icons after collapsing rocketbar

Status: RESOLVED WORKSFORME

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Alberto Pastor [:albertopq]]

Attachment: 2014-12-22-05-33-39.png

STR:

1.- Open the browser
2.- Click on a top site
3.- Collapse the rocketbar
4.- Click on the rocketbar
5.- Type 'firefox.com' (for example) and search
6.- After loaded, collapse the rocketbar again

Expected:

Icons not truncated

Actual:

Icons are truncated (see screenshot)

Comment 1

[GitHub Autolander Bot]

Attachment: [PullReq] albertopq:1114532-truncated-icons to mozilla-b2g:master

Comment 2

[Alberto Pastor [:albertopq]]

Before fixing the tests, I would like to hear Kevin's opinion here. Is there any case in which we want to display the back and forward buttons, when the chrome is collapsed? Probably I'm missing something about the context, as I don't get the css rules that are currently applied to the minimized chrome.

Comment 3

[Kevin Grandon :kgrandon]

I think it's probably fine to hide them. Can we simplify the change though and only add a display: none; for the buttons when it's collapsed?

Comment 4

[Alberto Pastor [:albertopq]]

I think only hiding the buttons would misplace the search input. We need to change the scaleX transforms as well

Comment 5

[Kevin Grandon :kgrandon]

Hmm, I don't think we should update the scaleX, why do we need to do that? Can you confirm that we are getting the correct width values in statusbar[1]? If so, then it seems like it might just be a layering or visibility issue?

[1] https://github.com/mozilla-b2g/gaia/blob/1f62cb4a1784f3605440192d068320c24ebf44e3/apps/system/js/statusbar.js#L617

Comment 6

[Alberto Pastor [:albertopq]]

Just to give an update, Kevin is going to take a look. Kevin, any update?

Comment 7

[Kevin Grandon :kgrandon]

Alberto - I'm not currently looking into this, but I could if needed. I would like to try to find a solution that does not involve adding the '.maximized' selector to all of the controls. Can you verify the details in comment 5, and see if there's any other solution beside adding that className to the selector? If not, I'll try to take a look, thanks.

Comment 8

[Alberto Pastor [:albertopq]]

Attachment: statusbar-back.png

This is how the rocketbar looks if we only display:none the buttons when the rocketbar is collapsed

Comment 9

[Alberto Pastor [:albertopq]]

(In reply to Kevin Grandon :kgrandon from comment #7)
> Alberto - I'm not currently looking into this, but I could if needed. I
> would like to try to find a solution that does not involve adding the
> '.maximized' selector to all of the controls. Can you verify the details in
> comment 5, and see if there's any other solution beside adding that
> className to the selector? If not, I'll try to take a look, thanks.

Hi Kevin, but I still don't follow you. If we just display:none the buttons when collapsed (as you suggest), the width calculation will be wrong (as the buttons will still be enabled). I added a screenshot on how it looks. Can we talk offline about it? Thanks!

Comment 10

[Kevin Grandon :kgrandon]

I think I'm fine with the transition, my main concern right now is adding to the already crazy CSS selectors for buttons. I want to avoid adding '.chrome.maximized' to each button if possible.

Comment 11

[Alberto Pastor [:albertopq]]

The only option I can think for making it work without restricting the [1] selectors to only when is maximized, is disabling the buttons when collapsing, so adding the disabled attribute to the buttons in [2], which in my opinion adds more complexity. May be I'm misunderstanding something. Happy to pass you the bug if you think is doable without using the maximized class (or adding a new class/attribute).


[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/style/chrome/chrome.css#L196
[2] https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/app_chrome.js#L695

Comment 12

[Kevin Grandon :kgrandon]

Is it really being truncated, or is the button just hovering over the element? It seems like we could potentially use a different z-index, or calculate the available width from another element. I'm more than happy to look at this once the tree re-opens.

Comment 13

[Alberto Pastor [:albertopq]]

I think the question is more, is resizing the search input depending on back and forward buttons (when the url bar is collapsed) part of the spec? As they are not visible, I think the search input width should remain the same all the time. Rob, Francis, if you collapse the rocketbar after navigating through the browser (so you see back button when is expanded), should the search input size be different than when there is no back button? That's what is currently happening.

Comment 14

[Kevin Grandon :kgrandon]

I can answer that - and no, it's not desired to be like that. What ends up happening is that the rocketbar shrinks down to be too small. I believe it's currently like that due to performance constraints.

It's something we should fix, but I also want to avoid adding yet another CSS class without removing one or more classes. I just don't want things to get too crazy as we have seen with other selectors.

I'll look at this as soon as the tree re-opens. I'm wondering if we can simplify the selector to:

.appWindow:not(.browser) .maximized .back-button:...

This would remove .chrome and .controls from the selector. We also need to be super careful about performance. Perhaps changing the width could cause a bit of jankiness during the rocketbar animation on lower-end devices?

Comment 15

[Alberto Pastor [:albertopq]]

Reassigning to Kevin, that knows the code better than me...

Comment 16

[Kevin Grandon :kgrandon]

I was unable to reproduce this issue using a different set of icons. I think the problem is more likely related to icon widths being wrongly defined in statusbar.js, but I need to verify that.

I'm adding qawanted here to see if we can reproduce and for branch checks.

Comment 17

[Pi Wei Cheng [:piwei] (inactive)]

I was unable to reproduce this issue on Flame 3.0 master and Flame 2.2.

I tried to make my status bar icons to match exactly with the screenshot provided at comment 0, which is having dual SIMs with one SIM roaming, connected to wifi, adb enabled, and charging. With this setup I followed the STR multiple attempts with no repro.

Device: Flame 3.0 (319mb, shallow flash)
Build ID: 20150126052534
Gaia: 793773bb2944b42a85dd160049e605cbd880c4da
Gecko: 95c76c3b0172
Version: 38.0a1 (3.0)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:38.0) Gecko/38.0 Firefox/38.0

Device: Flame 2.2 (319mb, shallow flash)
Build ID: 20150126125805
Gaia: 0518f4581a0925c0b703d730ef289ab15cbd1216
Gecko: b27709406790
Version: 37.0a2 (2.2)
Firmware Version: v18D-1
User Agent: Mozilla/5.0 (Mobile; rv:37.0) Gecko/37.0 Firefox/37.0

Leaving qawanted tag for others to attempt.

Comment 18

[Kevin Grandon :kgrandon]

Alberto - are you still able to reproduce this issue? I can't, it seems QA can't as well.

Comment 19

[Sue]

Attachment: screenshot.png

I was unable to reproduce this issue on Flame 2.1/2.2.
I have inserted dual SIMs, connected to wifi, adb enabled, and enable bluetooth and NFC. With this setup I followed the STR multiple attempts with no repro.
See attachment:screenshot.png
Rate:0/10

Flame 2.1 build:
Gaia-Rev        54d92cc0755e5102223276ab23063b5eee74b514
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/522d6c980917
Build-ID        20150126001827
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150126.041718
FW-Date         Mon Jan 26 04:17:29 EST 2015
Bootloader      L1TC000118D0

Flame 2.2 build:
Gaia-Rev        0518f4581a0925c0b703d730ef289ab15cbd1216
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/c6aa604a7967
Build-ID        20150126002536
Version         37.0a2
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20150126.042138
FW-Date         Mon Jan 26 04:21:49 EST 2015
Bootloader      L1TC000118D0

Comment 20

[Alberto Pastor [:albertopq]]

Yep, I can't repro it anymore. I still think that the Rocketbar input being smaller when we have back and/or forward buttons is a bug itself, but definitely a different one.

Thanks!

Comment 21

[Kevin Grandon :kgrandon]

Strange. Not sure what fixed it, but let's keep an eye on it to see if it comes back.