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)
Tracking
(blocking-b2g:2.1+, b2g-v2.1 verified, b2g-v2.2 verified)
People
(Reporter: gwagner, Assigned: apastor)
References
Details
(Whiteboard: [systemsfe])
Attachments
(3 files)
Open 2 apps in landscape and use edge gestures.
Using yelp and google maps for this example
Reporter | ||
Comment 1•10 years ago
|
||
[Blocking Requested - why for this release]:
blocking-b2g: --- → 2.1?
Reporter | ||
Updated•10 years ago
|
Whiteboard: [systemsfe]
Comment 2•10 years ago
|
||
Adding qawanted - Can we perform branch checks and look for a STR?
Keywords: qawanted
Comment 3•10 years ago
|
||
(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
Reporter | ||
Comment 4•10 years ago
|
||
This only happens if both apps have an extended rocketbar.
Comment 7•10 years ago
|
||
Can't reproduce it on master or on 2.1.
Maybe bug 1071983 fixed it?
Flags: needinfo?(etienne)
Reporter | ||
Comment 8•10 years ago
|
||
(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.
Comment 9•10 years ago
|
||
Etienne, I clearly remember showing you this bug :)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → apastor
Assignee | ||
Comment 10•10 years ago
|
||
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
Assignee | ||
Comment 11•10 years ago
|
||
Missed one step.
6.- Scroll down in order to show the RB on the statusbar
Assignee | ||
Comment 12•10 years ago
|
||
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
Assignee | ||
Comment 13•10 years ago
|
||
Attachment #8498179 -
Flags: review?(mhenretty)
Assignee | ||
Comment 14•10 years ago
|
||
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 15•10 years ago
|
||
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+
Comment 16•10 years ago
|
||
(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.
Comment 17•10 years ago
|
||
(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 18•10 years ago
|
||
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+
Assignee | ||
Comment 19•10 years ago
|
||
Comment on attachment 8498179 [details]
Link to Pull Request: https://github.com/mozilla-b2g/gaia/pull/24668
https://github.com/mozilla-b2g/gaia/pull/24668
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
Assignee | ||
Comment 20•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 21•10 years ago
|
||
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)
Comment 22•10 years ago
|
||
(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 23•10 years ago
|
||
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)
Comment 24•10 years ago
|
||
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 → ---
Comment 25•10 years ago
|
||
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.
Comment 26•10 years ago
|
||
My bad, the browser context menu doesn't live inside #dialog-overlay. Since app-window already takes
Comment 27•10 years ago
|
||
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.
Comment 28•10 years ago
|
||
... 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
Comment 29•10 years ago
|
||
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.
Assignee | ||
Comment 30•10 years ago
|
||
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)
Assignee | ||
Updated•10 years ago
|
Attachment #8499494 -
Flags: review?(kgrandon) → review?(etienne)
Comment 31•10 years ago
|
||
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+
Assignee | ||
Comment 32•10 years ago
|
||
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)
Comment 33•10 years ago
|
||
(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)
Updated•10 years ago
|
Target Milestone: --- → 2.1 S6 (10oct)
Assignee | ||
Comment 34•10 years ago
|
||
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•10 years ago
|
status-b2g-v2.1:
--- → affected
status-b2g-v2.2:
--- → fixed
Assignee | ||
Comment 35•10 years ago
|
||
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)
Updated•10 years ago
|
Attachment #8499494 -
Flags: approval-gaia-v2.1?(fabrice) → approval-gaia-v2.1+
Comment 36•10 years ago
|
||
Comment 37•10 years ago
|
||
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)
Updated•10 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Comment 38•10 years ago
|
||
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.
Description
•