Closed Bug 1034657 Opened 6 years ago Closed 6 years ago
Statusbar can get transparent in homescreen edit-mode after cancelled deletion of an app
233.27 KB, image/png
46 bytes, text/x-github-pull-request
|Details | Review|
46 bytes, text/x-github-pull-request
|Details | Review|
3.11 MB, video/mp4
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:30.0) Gecko/20100101 Firefox/30.0 (Beta/Release) Build ID: 20140611075517 Steps to reproduce: 1. Tap on an app icon until the edit-mode is entered. 2. Click on an an X to delete an app. 3. Click "delete" or "cancel". Don't leave the edit-mode. 4. Scroll to the top. Actual results: The statusbar gets transparent. Expected results: The statusbar should be opaque.
This occures in commit b37a4c3f
Can you test this on the latest 2.0? We've made a bunch of fixes on this recently, so this might no longer reproduce.
The issue persists in latest 2.0. I have found the issue before on the master branch.
Adding QA Wanted to see if someone else can reproduce & provide a screenshot.
The bug repros on Flame 2.1, Flame 2.0 and Buri 2.0 Flame 2.1 Build ID: 20140709070635 Gaia: c394b7b4205b6f1a6ca44915fc08650f3ad127ec Gecko: 2d88803a0b9c Platform Version: 33.0a1 Firmware Version: v122 User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0 Flame 2.0 Build ID: 20140709085028 Gaia: 3316542e36084a8d1f6a5446abe9bf199765f3d7 Gecko: 394b7c158cf0 Platform Version: 32.0a2 Firmware Version: v122 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0 Buri 2.0 Build ID: 20140709145735 Gaia: d9b225fd156b16adabcf459275b52674dde517cc Gecko: 32250091a0c7 Platform Version: 32.0a2 Firmware Version: v1.2device.cfg User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0 Actual result: After deleting an app (or not) on the homescreen. The status bar turns transparent if the user scrolls to the top of the homepage. -------------------------------------------------------------------------------------------------------- Bug does not repro on Flame 1.4 due to the fact that the vertical homescreen is not part of this version. Build ID: 20140709000201 Gaia: b0e9b4bdb39c5eb93a6783a34624ffc84f62b126 Gecko: acf704e54e19 Platform Version: 30.0 Firmware Version: v122 User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0
QA-Wanted report: not nomming, seems like a fringe-case
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Marked as 2.0? blocker as were the other transparent status bar bugs.
Status: UNCONFIRMED → NEW
blocking-b2g: --- → 2.0?
Ever confirmed: true
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][VH-FL-blocking-][VH-FC-blocking+]
Component: Gaia::System → Gaia::Homescreen
QA Whiteboard: [QAnalyst-Triage+][VH-FL-blocking-][VH-FC-blocking+] → [QAnalyst-Triage+][lead-review+][VH-FL-blocking-][VH-FC-blocking+]
I'll take a look here.
Assignee: nobody → mhenretty
Note, there is another bug here too. STR: 1.) Go into edit mode 2.) Scroll down from top of screen, and don't try to delete an app 3.) Click done 4.) Scroll back to top Actual Result: Statusbar stays opaque. Expected Result: Statusbar becomes transparent. It might be easier to fix both of these problems at once. If not, I will file a separate bug for the aforementioned issue.
Kevin, could you take a look?
Attachment #8454896 - Flags: review?(kgrandon)
Comment on attachment 8454896 [details] [review] Gaia PR, disable/enable confirm menu listeners in edit mode Simplified the code by removing what appears to be unneeded event listeners gaia-confirm-open/close. Also, out of curiosity Kevin, is there a reason you didn't use setAppearance in this commit ? Seems it could have just fallen through to the setAppearance call below, but perhaps I am missing something. 1.) https://github.com/mozilla-b2g/gaia/commit/b82a68967a60bae2ec650ec7d902344e7c4979d7#diff-8162f9191f6149ad503e9cf825234f38R55
(In reply to Michael Henretty [:mhenretty] from comment #12) > Also, out of curiosity Kevin, is there a reason you didn't use setAppearance > in this commit ? Seems it could have just fallen through to the > setAppearance call below, but perhaps I am missing something. > > 1.) > https://github.com/mozilla-b2g/gaia/commit/ > b82a68967a60bae2ec650ec7d902344e7c4979d7#diff- > 8162f9191f6149ad503e9cf825234f38R55 On visibilitychange you *always* want need to set the appearance, regardless of previous value. setAppearance caches and compares the value. It should probably be factored out into its own function.
Comment on attachment 8454896 [details] [review] Gaia PR, disable/enable confirm menu listeners in edit mode We do still unfortunately need these listeners =( There are other times that we use them when not in edit mode. One option might be to add/remove the listeners as the user enters or leaves edit mode, or check the current edit mode state. Also this could use a marionette test, probably in test/marionette/statusbar_test.
Attachment #8454896 - Flags: review?(kgrandon) → review-
Hey Mike, here's a test that we can land after we have a patch (assuming you still want to do this bug). This does unfortunately not cover the confirm dialog outside of edit mode, but I'll work on that next, or potentially as part of another bug.
(In reply to Kevin Grandon :kgrandon from comment #15) > Created attachment 8454988 [details] [review] > Pull request - integration test for confirm dialog in edit mode > > Hey Mike, here's a test that we can land after we have a patch (assuming you > still want to do this bug). This does unfortunately not cover the confirm > dialog outside of edit mode, but I'll work on that next, or potentially as > part of another bug. Just wanted to comment to say that I've added another test case which checks the confirm dialog outside of edit mode. Assuming both tests pass we should be good. Thanks!
Kevin, this mostly works with your integration tests. There is one problem, but I think it may be a typo in the integration test. I left a comment on attachment 8454988 [details] [review] about that.
Comment on attachment 8455686 [details] [review] Github PR, remove gaia-confirm listener when in edit mode Looks good, thanks! Master: https://github.com/mozilla-b2g/gaia/commit/16ce0675648e52848599812165bdc2b8e8f1d895
Attachment #8455686 - Flags: review?(kgrandon) → review+
Comment on attachment 8454988 [details] [review] Pull request - integration test for confirm dialog in edit mode Hey Mike, can you put a review stamp on the test? Thanks!
Attachment #8454988 - Flags: review?(mhenretty)
Comment on attachment 8454988 [details] [review] Pull request - integration test for confirm dialog in edit mode The test itself looks great. Gaia-try is failing the new test though, but perhaps that is because it ran before merging me patch? Kevin, can you force push to your branch again to re-run gaia try? (I don't think re-triggering the tests will pick up my changes, but I could be wrong)
Attachment #8454988 - Flags: review?(mhenretty) → review+
Let's land for now to get the uplift. The test is still red, but if I can't get a green build soon I'll move it into a follow-up.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Oops, forgot to land the test, green builds on tbpl/travis: https://github.com/mozilla-b2g/gaia/commit/c8392728c3bf610b9d746ddf922fb06893bff8c0
for the test commit: v2.0: 3b61868a7f2817ef73b1c9303c072dcd6462a233
According to the description and the comment#10, the issue has been verified successfully on Flame 2.0 and 2.1. See attachment: Verify_1034657.MP4 Reproducing rate: 0/5 Flame 2.0 build: Gaia-Rev 856863962362030174bae4e03d59c3ebbc182473 Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/e40fe21e37f1 Build-ID 20141208000206 Version 32.0 Flame 2.1 build: Gaia-Rev 38e17b0219cbc50a4ad6f51101898f89e513a552 Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/8b92c4b8f59a Build-ID 20141205001201 Version 34.0
You need to log in before you can comment on or make changes to this bug.