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)

x86_64
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 31
Tracking Status
firefox29 --- affected
firefox30 --- affected
firefox31 --- verified

People

(Reporter: cbadau, Assigned: Paenglab)

References

Details

Attachments

(1 file, 2 obsolete files)

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.
OS: All → Windows 7
Hardware: All → x86
Hardware: x86 → x86_64
Blocks: 718011
Blocks: 738796
Attached patch inContentShowTab.patch (obsolete) (deleted) — Splinter Review
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.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8404071 - Flags: review?(mak77)
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+
Attached patch inContentShowTab.patch (obsolete) (deleted) — Splinter Review
Addressed comments.
Attachment #8404071 - Attachment is obsolete: true
Attachment #8404610 - Flags: review?(mak77)
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+
Attached patch Patch for check-in (deleted) — Splinter Review
Patch for check-in addressed the comments.
Attachment #8404610 - Attachment is obsolete: true
Attachment #8404958 - Flags: review+
Keywords: checkin-needed
(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...
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 31
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.

Attachment

General

Created:
Updated:
Size: