Closed Bug 1074033 Opened 10 years ago Closed 10 years ago

Overlapping status bar icons during landscape edge gesture transition

Categories

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

x86
macOS
defect
Not set
normal

Tracking

(blocking-b2g:2.1+, b2g-v2.1 verified, b2g-v2.2 verified)

VERIFIED FIXED
2.1 S6 (10oct)
blocking-b2g 2.1+
Tracking Status
b2g-v2.1 --- verified
b2g-v2.2 --- verified

People

(Reporter: gwagner, Assigned: apastor)

References

Details

(Whiteboard: [systemsfe])

Attachments

(3 files)

Attached image transition (deleted) —
Open 2 apps in landscape and use edge gestures. Using yelp and google maps for this example
[Blocking Requested - why for this release]:
blocking-b2g: --- → 2.1?
Whiteboard: [systemsfe]
Adding qawanted - Can we perform branch checks and look for a STR?
Keywords: qawanted
(In reply to Kevin Grandon :kgrandon from comment #2) > Adding qawanted - Can we perform branch checks and look for a STR? Nevermind, I had the wrong bug open. I think this one is pretty clear, so removing qawanted. Feel free to add it if we still need it.
Keywords: qawanted
This only happens if both apps have an extended rocketbar.
Bad UX.
blocking-b2g: 2.1? → 2.1+
Etienne, can you take this one?
Flags: needinfo?(etienne)
Can't reproduce it on master or on 2.1. Maybe bug 1071983 fixed it?
Flags: needinfo?(etienne)
(In reply to Etienne Segonzac (:etienne) from comment #7) > Can't reproduce it on master or on 2.1. > Maybe bug 1071983 fixed it? Still happening but it took me now 5 min to reproduce. This time with swiping between cutTheRope and the CNN webpage.
Etienne, I clearly remember showing you this bug :)
Assignee: nobody → apastor
I found stable STR: 1.- Open the Browser 2.- Go to any page (let's call it page 1) 3.- Click on the '...' next to the url bar, and open a new window 4.- Go to any page (let's call it page 2) 5.- Swipe back to page 1 Expected Every swipe from page 1 should hide the statusbar Actual When swiping (in any direction and any orientation) it doesn't hide the statusbar on the top right
Missed one step. 6.- Scroll down in order to show the RB on the statusbar
Actually, there is not even need to open a 2nd page. STR 1.- Open the Browser 2.- Go to any page 3.- Click on the '...' next to the url bar 4.- Click close on the dialog 5.- Scroll down in order to show the RB on the statusbar
Tricky one... Apparently, the dialog shown when clicking '...' next to the url bar remains in the DOM (although with visibility hidden) after clicking Close. Not sure why, but that's making the app chrome receiving a scroll event on edge gestures, which makes the statusbar visible after the 'sheets-gesture-begin' event. Absolute positioning the dialog makes the chrome not receiving that event anymore (in the same way it does when the form is not in the DOM). Probably a platform bug somewhere...?
Comment on attachment 8498179 [details] Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/24668 This fixes the bug for me, so fb+, but I can't review this as I'm not familiar enough with the action menu code. That said, I wonder why we ever show the real statusbar at all? In my mind, we only ever display the moz-elements attached to the app window, and the real status bar should be hidden somewhere without ever showing his face.
Attachment #8498179 - Flags: review?(mhenretty)
Attachment #8498179 - Flags: review?(alive)
Attachment #8498179 - Flags: feedback+
(In reply to Michael Henretty [:mhenretty] from comment #15) > Comment on attachment 8498179 [details] > Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/24612 > > This fixes the bug for me, so fb+, but I can't review this as I'm not > familiar enough with the action menu code. > > That said, I wonder why we ever show the real statusbar at all? In my mind, > we only ever display the moz-elements attached to the app window, and the > real status bar should be hidden somewhere without ever showing his face. I have the same feeling. This fix looks odd to this issue.
(In reply to Michael Henretty [:mhenretty] from comment #15) > That said, I wonder why we ever show the real statusbar at all? So alberto had already did a good investigation in comment 14. When starting swipe navigation, statusbar will hide due to sheet-transition-start event; but it will soon get another apptitlechanged event due to appChrome thinks the browser container is scrolling. What's more strange is this could only happen if the statusbar is transfering from maximized to miminized.
Comment on attachment 8498179 [details] Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/24668 This looks like a scrollgrab bug but r+ first. On the other hand, either in statusbar or in appChrome we should not dispatch titlechanged event while we are doing sheet navigation, but this is not easy before sheet transition is moving into AppWindow.
Attachment #8498179 - Flags: review?(alive) → review+
Attachment #8498179 - Attachment description: Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/24612 → Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/24668
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Comment on attachment 8498179 [details] Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/24668 [Approval Request Comment] [Bug caused by] (feature/regressing bug #): - [User impact] if declined: Statusbar icons overlap when edge swiping [Testing completed]: Manual testing [Risk to taking this patch] (and alternatives if risky): Low risk. CSS only [String changes made]: -
Attachment #8498179 - Flags: approval-gaia-v2.1?(fabrice)
(In reply to Alberto Pastor [:albertopq] from comment #14) > Tricky one... Apparently, the dialog shown when clicking '...' next to the > url bar remains in the DOM (although with visibility hidden) after clicking > Close. Not sure why, but that's making the app chrome receiving a scroll > event on edge gestures, which makes the statusbar visible after the > 'sheets-gesture-begin' event. Absolute positioning the dialog makes the > chrome not receiving that event anymore (in the same way it does when the > form is not in the DOM). Probably a platform bug somewhere...? wow. Thanks Alberto!
Comment on attachment 8498179 [details] Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/24668 Needs a fix before uplift.
Attachment #8498179 - Flags: approval-gaia-v2.1?(fabrice)
I've reverted this for breaking the contextmenu in the browser chrome and causing bug 1077003. If we get a review for bug 1077003 we can land these together, or you can squash that fix into the pull request here. Either way works for me. I didn't see you on IRC so I'll follow-up with Mike and Eli to see if we can get bug 1077003 in as well. https://github.com/mozilla-b2g/gaia/commit/2264fd0b71a70fa59d1da829f8a5d31ef86d40ee
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Depends on: 1077003
I closed bug 1077003 since we reverted the patch here. It's not entirely clear to me why this patch caused that bug. It's possible that bug 1077003 is caused by our resizing .dialog-overlay in the system, and not related to this patch. In any case, let's make sure bug 1077003 isn't still happening before relanding this.
My bad, the browser context menu doesn't live inside #dialog-overlay. Since app-window already takes
Alberto - Feel free to take the patch from bug 1077003 if you'd like (https://github.com/mozilla-b2g/gaia/pull/24716), and you can squash it into your commit. Not sure if there is a better fix.
... soft home button into account, setting position absolute (from fixed) made the SHB correction in action_menu_extended [1] incorrect. So if we change the positioning for general dialogs like that, we need to remove that correction. 1.) https://github.com/mozilla-b2g/gaia/blob/2264fd0b71a70fa59d1da829f8a5d31ef86d40ee/apps/system/style/action_menu/action_menu_extended.css#L3
In any case, we need to be very careful how we fix this. Changing positioning across all "action dialogs" can have unforeseen ramifications, so let's just make sure we know what we are getting into.
I made the change for only affecting the dialog when hidden, so it should have not any side effects. I tested it with the soft button and looks fine.
Attachment #8499494 - Flags: review?(kgrandon)
Attachment #8499494 - Flags: review?(kgrandon) → review?(etienne)
Comment on attachment 8499494 [details] Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/24751 This indeed fixes the issue, kudos. r=me with - a comment explaining why we're doing this - a gecko bug filed and referenced (so we remember to make a smaller test case showing the issue) And finally, to stop the madness of bug cross-over can you make sure that in the |@media (orientation: landscape)| media query at the top of the file you're changing we set |bottom:0| |right: var(--software-home-button-height)| for |#screen.software-button-enabled [role="dialog"][data-type="action"]| and |#screen.software-button-enabled [role="dialog"][data-type="object"]|. Thanks!
Attachment #8499494 - Flags: review?(etienne) → review+
Thanks for the review, Etienne! I added a comment and filed a bug. Regarding landscape, good catch! When the Soft button is enabled, the browser dialog is not correctly displayed. I tried to add what you where proposing, but apparently is not taking the media query. I can investigate more, but don't you think it makes more sense to fix that on Bug 1074123? I think the problem you are pointing out is still there with or without this patch so, are you happy on landing this and keeping track of the landscape problem on another bug? Thanks!
Flags: needinfo?(etienne)
(In reply to Alberto Pastor [:albertopq] from comment #32) > Thanks for the review, Etienne! > > I added a comment and filed a bug. > Regarding landscape, good catch! When the Soft button is enabled, the > browser dialog is not correctly displayed. I tried to add what you where > proposing, but apparently is not taking the media query. It works perfectly for me (just tried). > I can investigate > more, but don't you think it makes more sense to fix that on Bug 1074123? Yes don't block on it.
Flags: needinfo?(etienne)
Target Milestone: --- → 2.1 S6 (10oct)
Status: REOPENED → RESOLVED
Closed: 10 years ago10 years ago
Resolution: --- → FIXED
Comment on attachment 8499494 [details] Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/24751 [Approval Request Comment] [Bug caused by] (feature/regressing bug #): [User impact] if declined: Broken UX. The statusbar is doubled on edge transitions [Testing completed]: Manual tests. [Risk to taking this patch] (and alternatives if risky): Low risk. CSS that only affects to a hidden div. [String changes made]: -
Attachment #8499494 - Flags: approval-gaia-v2.1?(fabrice)
Attachment #8499494 - Flags: approval-gaia-v2.1?(fabrice) → approval-gaia-v2.1+
This issue no longer occurs on, Flame 2.2 Master KK (319mb) (Full Flash) and flame 2.1 KK (319mb) (Full Flash) the overlapping no longer occurs from the 2 apps in landscape mode being edge swiped between each other. Flame 2.2 Master KK (319mb) (Full Flash) Device: Flame 2.2 Master BuildID: 20141011040204 Gaia: 95f580a1522ffd0f09302372b78200dab9b6f322 Gecko: 3f6a51950eb5 Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf Version: 35.0a1 (2.2 Master) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0 2.1 KK (319mb) (Full Flash) Environmental Variables: Device: Flame 2.1 KK (319mb) (Full Flash) Build ID: 20141010000201 Gaia: d71f8804d7229f4b354259d5d8543c25b4796064 Gecko: 7fa82c9acdf2 Version: 34.0a2 Flame 2.1 KK (319mb) Firmware Version: v180 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Setting testsuite- for bugs which depend on landscape b2g desktop for testing. This list will be re-triaged once support is added.
Depends on: 1080309
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: