Closed
Bug 993344
Opened 11 years ago
Closed 11 years ago
The "Show tab previews in the Windows taskbar" option doesn't appear in Tabs section for in-content preferences
Categories
(Firefox :: Settings UI, defect)
Tracking
()
VERIFIED
FIXED
Firefox 31
People
(Reporter: cbadau, Assigned: Paenglab)
References
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
Paenglab
:
review+
|
Details | Diff | Splinter Review |
Reproducible on the latest Beta (BuildID:20140407135746)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20130205 Firefox/29.0
Reproducible on the latest Aurora (BuildID:20140407004002)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:30.0) Gecko/20130205 Firefox/30.0
Reproducible on the latest Nightly (BuildID: 20140407030203)
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0
Steps to reproduce:
1. Open about:preferences.
2. Go to General tab, on Tabs section.
3. Observe the options displayed in this section.
Actual results: The "Show tab previews in the Windows taskbar" option (which is visible under Tools->Options->Tabs) doesn't appear in Tabs section.
Expected results: All the options displayed under Tools -> Options -> Tabs appear in Tabs section (from General tab).
Notes: This issue is not a regression.
Reporter | ||
Updated•11 years ago
|
Reporter | ||
Updated•11 years ago
|
OS: All → Windows 7
Hardware: All → x86
Reporter | ||
Updated•11 years ago
|
Hardware: x86 → x86_64
Assignee | ||
Comment 1•11 years ago
|
||
Made the version selector similar to the prefwindow's selector.
http://mxr.mozilla.org/comm-central/source/mozilla/browser/components/preferences/tabs.js#41
Also changed the misleading comment from Win XP to Windows 7 and up and added the prefs name with description.
Comment 2•11 years ago
|
||
Comment on attachment 8404071 [details] [diff] [review]
inContentShowTab.patch
Review of attachment 8404071 [details] [diff] [review]:
-----------------------------------------------------------------
the fix looks ok, the surrounding may get some additional fixes. I'll take a second look at the fixed patch.
Thanks.
::: browser/components/preferences/in-content/main.js
@@ +22,5 @@
> .getService(Components.interfaces.nsIObserverService)
> .notifyObservers(window, "main-pane-loaded", null);
>
> +#ifdef XP_WIN
> + //Functionality for "Show tabs in taskbar" on Windows 7 and up
please add space after // and end comment with a period, like:
// Show "Show tabs in taskbar" option on Windows 7 and up.
@@ +23,5 @@
> .notifyObservers(window, "main-pane-loaded", null);
>
> +#ifdef XP_WIN
> + //Functionality for "Show tabs in taskbar" on Windows 7 and up
> +
trailing spaces here
@@ +26,5 @@
> + //Functionality for "Show tabs in taskbar" on Windows 7 and up
> +
> + /* browser.taskbar.previews.enable
> + * - true if tabs are to be shown in the Windows 7 taskbar
> + */
This comment should be moved below, after // TABS, check what we do in case of DOWNLOADS, just down the same file:
// DOWNLOADS
/*
* Preferences:
*
* browser.download.useDownloadDir - bool
...
you should basically do the same for // TABS, bonus points if you also add documentation for the other TABS options
@@ +33,5 @@
> let sysInfo = Cc["@mozilla.org/system-info;1"].
> getService(Ci.nsIPropertyBag2);
> let ver = parseFloat(sysInfo.getProperty("version"));
> let showTabsInTaskbar = document.getElementById("showTabsInTaskbar");
> + showTabsInTaskbar.hidden = ver < 6.1;
The fix looks ok, the bug was introduced when tabs options were merged back into General, someone forgot to remove the history.state check.
Attachment #8404071 -
Flags: review?(mak77) → feedback+
Assignee | ||
Comment 3•11 years ago
|
||
Addressed comments.
Attachment #8404071 -
Attachment is obsolete: true
Attachment #8404610 -
Flags: review?(mak77)
Comment 4•11 years ago
|
||
Comment on attachment 8404610 [details] [diff] [review]
inContentShowTab.patch
Review of attachment 8404610 [details] [diff] [review]:
-----------------------------------------------------------------
Just some wording may be improved. r=me with that.
::: browser/components/preferences/in-content/main.js
@@ +442,5 @@
> option.removeAttribute("disabled");
> startupPref.updateElements(); // select the correct index in the startup menulist
> }
> },
> // TABS
please add a newline before // TABS
@@ +448,5 @@
> + /*
> + * Preferences:
> + *
> + * browser.link.open_newwindow - int
> + * Determines where pages which would open in a new window are opened.
Determines where links targeting new windows should open.
@@ +450,5 @@
> + *
> + * browser.link.open_newwindow - int
> + * Determines where pages which would open in a new window are opened.
> + * Values:
> + * 1 - Opens such links in the most recent window or tab.
Open in the current window or tab.
@@ +451,5 @@
> + * browser.link.open_newwindow - int
> + * Determines where pages which would open in a new window are opened.
> + * Values:
> + * 1 - Opens such links in the most recent window or tab.
> + * 2 - Opens such links in a new window.
Open in a new window.
@@ +452,5 @@
> + * Determines where pages which would open in a new window are opened.
> + * Values:
> + * 1 - Opens such links in the most recent window or tab.
> + * 2 - Opens such links in a new window.
> + * 3 - Opens such links in a new tab.
Open in a new tab in the most recent window.
@@ +455,5 @@
> + * 2 - Opens such links in a new window.
> + * 3 - Opens such links in a new tab.
> + * browser.tabs.loadInBackground - bool
> + * True - If display should switch to a new tab which has been opened from
> + * a link, false if display shouldn't switch.
Whether browser should switch to a new tab opened from a link.
@@ +458,5 @@
> + * True - If display should switch to a new tab which has been opened from
> + * a link, false if display shouldn't switch.
> + * browser.tabs.warnOnClose - bool
> + * True - If when closing a window with multiple tabs the user is warned and
> + * allowed to cancel the action, false to just close the window.
Whether the user should be warned when trying to close a window with multiple tabs, allowing to cancel the action.
@@ +461,5 @@
> + * True - If when closing a window with multiple tabs the user is warned and
> + * allowed to cancel the action, false to just close the window.
> + * browser.tabs.warnOnOpen - bool
> + * True - If the user should be warned if he attempts to open a lot of tabs
> + * at once (e.g. a large folder of bookmarks).
Whether the user should be warned when trying to open a lot of tabs at once (e.g. a large folder of bookmarks), allowing to cancel the action.
Attachment #8404610 -
Flags: review?(mak77) → review+
Assignee | ||
Comment 5•11 years ago
|
||
Patch for check-in addressed the comments.
Attachment #8404610 -
Attachment is obsolete: true
Attachment #8404958 -
Flags: review+
Assignee | ||
Updated•11 years ago
|
Keywords: checkin-needed
Comment 6•11 years ago
|
||
(In reply to Camelia Badau, QA [:cbadau] from comment #0)
> Notes: This issue is not a regression.
Based on the patch, this looks like a regression from bug 767313? The patch in that bug changed between bug 767313 comment 4 and bug 767313 comment 6 to include the code removed in this patch, without explanation. I wonder if Manish remembers why...
Comment 7•11 years ago
|
||
Keywords: checkin-needed
Comment 8•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
Reporter | ||
Comment 9•11 years ago
|
||
Verified fixed on Windows 7 x86_64 using Nightly 31.0a1 (buildID: 20140428030203) and the latest Nightly 32.0a1 (buildID: 20140501030202).
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•