Closed Bug 1014238 Opened 7 years ago Closed 7 years ago
.ui Customization .state is set to something bogus, Back/Forward functionality becomes completely broken . (and hamburger button & customize context-menu-entry have no effect)
screencast (showing back button being non-functional, aside from its context-menu, & broken hamburger-menu & customize mode.)
1.56 MB, video/ogg
1.40 KB, patch
|Details | Diff | Splinter Review|
STR: 1. Set pref browser.uiCustomization.state to "", or to "invalid", or whatever you like. 2. Restart browser 3. Click through a few links/pages. Try to open hamburger menu. Try to open customize mode (e.g. by right-clicking toolbar and picking "Customize") ACTUAL RESULTS: Firefox superficially *looks* normal, BUT: a) Back button is visible but disabled (non-functional). Forward button is never shown. b) Can't hack around (a) by right-clicking on a page and picking "back"; that context-menu-entry is disabled too. c) Clicking hamburger button has no effect. d) Picking "Customize" from toolbar context-menu has no effect. EXPECTED RESULTS: At a bare minimum, back/forward functionality should work (via toolbar buttons, as long as we're showing them, and *definitely* via context menu). Ideally, I'd expect hamburger/customize-mode to be functional, too.
(Note: this might seem to be an edge case not worth supporting, but I think I might have gotten myself into a variant of this state, without actively tweaking this pref. Digging a bit further, but running out of the time I allocated to investigating this.)
(Strangely, if I *right-click* the back button, I do see my browsing history for the given tab, & I can navigate back or forward that way. But the button is otherwise broken -- it renders as disabled and left-clicking it has no effect.)
Wat. I was so sure that we caught this case; there's a try catch for the JSON reading call here. But it clearly doesn't work well - on OS X I can trigger much worse breakage than this, even.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
For completeness, I'm testing with today's nightly: Mozilla/5.0 (X11; Linux x86_64; rv:32.0) Gecko/20100101 Firefox/32.0 32.0a1 (2014-05-21) Built from https://hg.mozilla.org/mozilla-central/rev/9d8d16695f6a
This fixes things, AFAICT. Sadly not really possible to write a good test for this...
Attachment #8426590 - Flags: review?(jaws)
Comment on attachment 8426590 [details] [diff] [review] don't break on invalid customization state, Review of attachment 8426590 [details] [diff] [review]: ----------------------------------------------------------------- r=me
Attachment #8426590 - Flags: review?(jaws) → review+
remote: https://hg.mozilla.org/integration/fx-team/rev/47b33dc2a235 Guess this happened this iteration, too, Marco. :-( / :-)
Added to Iteration 32.2
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team] p=1 s=it-32c-31a-30b.2 [qa+] → p=1 s=it-32c-31a-30b.2 [qa+]
Target Milestone: --- → Firefox 32
I verified the bug using the following environment: FF 32 Nightly Build Id: 20140523030202 Os: Win 7 x64, Ubuntu 12.10 x32, Mac Os X 10.8.5 When browser.uiCustomization.state is set to a bogus value it will be changed to the default value after the restart. I was wondering if it shouldn't be set to the last valid value?
Whiteboard: p=1 s=it-32c-31a-30b.2 [qa+] → p=1 s=it-32c-31a-30b.2 [qa!]
(In reply to Catalin Varga [QA][:VarCat] from comment #11) > I verified the bug using the following environment: > > FF 32 Nightly > Build Id: 20140523030202 > Os: Win 7 x64, Ubuntu 12.10 x32, Mac Os X 10.8.5 > > When browser.uiCustomization.state is set to a bogus value it will be > changed to the default value after the restart. I was wondering if it > shouldn't be set to the last valid value? We have no way to do that, because we don't know what the last valid value is/was.
Marking as Verified for Nightly based on the previous 2 comments.
What's the risk to taking this to Beta? We've got one left on Thursday and this looks like a simple fix to a potentially painful user experience. Please nominate with risk assessment.
Comment on attachment 8426590 [details] [diff] [review] don't break on invalid customization state, [Approval Request Comment] Bug caused by (feature/regressing bug #): n/a User impact if declined: potential total breakage if pref value for ui.customizationState gets messed up for whatever reason Testing completed (on m-c, etc.): on m-c for a long time Risk to taking this patch (and alternatives if risky): zero to none String or IDL/UUID changes made by this patch: none
Marking back as [qa+] for verification in next Beta and Aurora.
Whiteboard: p=1 s=it-32c-31a-30b.2 [qa!] → p=1 s=it-32c-31a-30b.2 [qa+]
Verified as fixed using Firefox 30 beta 9 and latest Aurora on Windows 8.1 64bit, Ubuntu 13.04 64bit and Mac OS X 10.9.2.
You need to log in before you can comment on or make changes to this bug.