Closed Bug 1365912 Opened 3 years ago Closed 3 years ago

Add hidden pref for enabling compact/touch theme modes

Categories

(Firefox :: Toolbars and Customization, enhancement, P1)

enhancement

Tracking

()

VERIFIED FIXED
Firefox 55
Iteration:
55.7 - Jun 12
Tracking Status
firefox55 --- verified

People

(Reporter: dao, Assigned: johannh)

References

Details

(Whiteboard: [photon-visual][p1])

Attachments

(1 file)

We can do this before we've settled on how the UI for this pref should look like.
Flags: qe-verify+
Priority: -- → P2
QA Contact: brindusa.tot
Component: General → Toolbars and Customization
Blocks: 1355771
I'll take this.
Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Iteration: --- → 55.6 - May 29
Priority: P2 → P1
Blocks: 1367421
No longer blocks: 1367421
Iteration: 55.6 - May 29 → 55.7 - Jun 12
Two things I'm not sure about:

- I need to add the attribute in onLoad before the window is shown, could setting the attribute still negatively affect performance?
- Does this need a test? Any suggestions where to put it except b/b/c/test/general?
Comment on attachment 8872596 [details]
Bug 1365912 - Add a pref for enabling compact and touch mode.

https://reviewboard.mozilla.org/r/144136/#review147882

::: browser/base/content/browser.js:1556
(Diff revision 1)
>      gBrowser.tabContainer.updateVisibility();
>  
>      BookmarkingUI.init();
>      AutoShowBookmarksToolbar.init();
>  
> +    gPrefService.addObserver(gUIDensity.prefDomain, gUIDensity);

This should happen together with the first update, otherwise code opening a new window and setting the pref might create a race condition.
Comment on attachment 8872596 [details]
Bug 1365912 - Add a pref for enabling compact and touch mode.

https://reviewboard.mozilla.org/r/144136/#review147886

::: browser/base/content/browser.js:1839
(Diff revision 2)
>        } catch (ex) {
>          Cu.reportError(ex);
>        }
>  
> +      try {
> +        gPrefService.removeObserver(gUIDensity.prefDomain, gUIDensity);

Need to do this regardless of this._boundDelayedStartup.

Also, this isn't really expected to throw, is it?
Comment on attachment 8872596 [details]
Bug 1365912 - Add a pref for enabling compact and touch mode.

https://reviewboard.mozilla.org/r/144136/#review147886

> Need to do this regardless of this._boundDelayedStartup.
> 
> Also, this isn't really expected to throw, is it?

Argh, good catch. And I have no idea if this is throwing, the other thing that registered a pref observer did it, but there's no indication that this actually throws, so I'll remove it.
This was probably expecting an exception in case delayedStartup didn't run before the window would close, but the _boundDelayedStartup check is taking care of that these days.
Comment on attachment 8872596 [details]
Bug 1365912 - Add a pref for enabling compact and touch mode.

https://reviewboard.mozilla.org/r/144136/#review147930
Attachment #8872596 - Flags: review?(dao+bmo) → review+
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/77756ce1743d
Add a pref for enabling compact and touch mode. r=dao
https://hg.mozilla.org/mozilla-central/rev/77756ce1743d
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
I can see the feature implemented in Nightly 55.0a1 on Windows 10, 64-bit

Build ID 	20170612030208
User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0
QA Whiteboard: [bugday-20170614]
I can see the feature implemented in Nightly 55.0a1 on Linux Mint 18.1 Serena"(64 Bit)

Build ID 	20170611100318
User Agent 	Mozilla/5.0 (X11; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0

[Bugday-20170614]
As per Comment 13 & Comment 14, I am marking this bug as verified fixed.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Duplicate of this bug: 1375932
You need to log in before you can comment on or make changes to this bug.