Closed Bug 911830 Opened 7 years ago Closed 7 years ago
Regression: About:Home completely blank after exiting out of search results screen
Currently the entirety of about:home is blank after backing out of the search suggestions screen. Steps to Reproduce On a new profile i) Tap the address-bar ii) Enter 'Cat' and accept search suggestions iii) Hit back twice See screenshot Only possibly related console output D/AbsListView(16555): onVisibilityChanged() is called, visibility : 4 D/AbsListView(16555): unregisterIRListener() is called D/AbsListView(16555): unregisterIRListener() is called D/AbsListView(16555): onDetachedFromWindow -- Nightly (09/02) LG Nexus 4 (Android 4.3), Samsung Galaxy SIV (Android 4.3), Samsung Galaxy Note II (Android 4.2)
Closing the active about:home tab does not restore about:home to a working state. State is restored after viewing a new website. Accessing the URL bar does not redisplay about:home. This is a major loss of functionality.
Severity: normal → major
Hardware: x86 → ARM
Regression from here: https://hg.mozilla.org/integration/fx-team/rev/e355c7aae965
(In reply to Mark Finkle (:mfinkle) from comment #4) > Comment on attachment 799106 [details] [diff] [review] > Patch > > This seems broken: show it! hide it! Hide doesnt hide it if its about:home. Basically setVisibility() used by the regression patch is wrong. We need to clean that up. filterEditMode() is causing all the problems.
(In reply to Sriram Ramasubramanian [:sriram] from comment #5) > (In reply to Mark Finkle (:mfinkle) from comment #4) > > Comment on attachment 799106 [details] [diff] [review] > > Patch > > > > This seems broken: show it! hide it! > > Hide doesnt hide it if its about:home. Basically setVisibility() used by the > regression patch is wrong. We need to clean that up. filterEditMode() is > causing all the problems. It sounds like you have a better fix in mind... How hard would it be to just do that here, rather than land this workaround?
I was thinking about it. But that too basically does the same thing by calling a show(), hide() method. Basically the regression changeset hides the visibility of about:home. This should be restored on dismissing the url-edit. That's what my patch does.
Comment on attachment 799106 [details] [diff] [review] Patch Okay, well let's add a comment here explaining that this reverses what we're doing in filterEditingMode. Maybe those changes should have gone in hideBrowserSearch/showBrowserSearch? Although hideBrowserSearch is called in a bunch of places, so maybe we don't want that logic everywhere. Regardless, I think this logic needs a bit more organization to feel intuitive.
Attachment #799106 - Flags: review?(margaret.leibovic) → review+
How bad is the problem the regressing code fixed? I'd consider backing out the regressing code and finding a new way to fix it. This is not a good patch.
Adding maxli: How bad is the problem that you fixed in Bug 906670? Shall we back that out or do something that doesn't involve setVisibility()?
(In reply to Sriram Ramasubramanian [:sriram] from comment #10) > Adding maxli: > How bad is the problem that you fixed in Bug 906670? Shall we back that out > or do something that doesn't involve setVisibility()? I'd say it's relatively major (e.g. see Marco's statements in Bug 906670 comment 3). I'm not too sure what else you could do to fix bug 906670 besides changing the visibility. As far as I know, the only other thing you could do would be to override the onInitializeAccessibilityNodeInfo method of the HomePager to say it's invisible when it's not shown, but to actually do that you'd probably still have to have essentially the same calls as the existing patch to maintain that state. So I don't think that's really a better solution.
I would be fine with the bandaid fix in this patch, as long as we file a followup bug to fix it better!
Assignee: nobody → sriram
tracking-fennec: ? → 26+
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 26
Friendly reminder to file the follow-up bug before we forget :)
You need to log in before you can comment on or make changes to this bug.