User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8) Gecko/20051111 Firefox/1.5 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060104 Firefox/1.6a1 - Build ID: 2006010405 The given URL has some default styles and an alternate stylesheet for handhelds When filling the "View->Page Style" menu Firefox thinks, he doesn't have to show the entry "default page style" because there is a preferred stylesheet set (a stylesheet with title attribute). Unfortunately the preferred stylesheet has media="handheld" so it is not displayed in the menu and Firefox isn't displaying this the menu, too. Reproducible: Always Steps to Reproduce: 1. Load given URL 2. Open menu View->Page Style Actual Results: There is only an entry for "No Style" if you select "No Style", you can't switch back Expected Results: There should be the menuitem "Default Style" The problem is in "function stylesheetFillPopup(menuPopup)" in browser.js Here is a fixed version of the function, i've commented by changes: --------------------- function stylesheetFillPopup(menuPopup) { var noStyle = menuPopup.firstChild; var persistentOnly = noStyle.nextSibling; var sep = persistentOnly.nextSibling; while (sep.nextSibling) menuPopup.removeChild(sep.nextSibling); var styleSheets = getAllStyleSheets(window.content); var currentStyleSheets = []; var styleDisabled = getMarkupDocumentViewer().authorStyleDisabled; var haveAltSheets = false; var altStyleSelected = false; var altSheetsAdded = false; for (var i = 0; i < styleSheets.length; ++i) { var currentStyleSheet = styleSheets[i]; // Skip any stylesheets that don't match the screen media type. var media = currentStyleSheet.media.mediaText.toLowerCase(); if (media && (media.indexOf("screen") == -1) && (media.indexOf("all") == -1)) continue; if (currentStyleSheet.title) { if (!currentStyleSheet.disabled) altStyleSelected = true; haveAltSheets = true; var lastWithSameTitle = null; if (currentStyleSheet.title in currentStyleSheets) lastWithSameTitle = currentStyleSheets[currentStyleSheet.title]; if (!lastWithSameTitle) { var menuItem = document.createElement("menuitem"); menuItem.setAttribute("type", "radio"); menuItem.setAttribute("label", currentStyleSheet.title); menuItem.setAttribute("data", currentStyleSheet.title); menuItem.setAttribute("checked", !currentStyleSheet.disabled && !styleDisabled); menuPopup.appendChild(menuItem); altSheetsAdded = true; currentStyleSheets[currentStyleSheet.title] = menuItem; } else { if (currentStyleSheet.disabled) lastWithSameTitle.removeAttribute("checked"); } } } noStyle.setAttribute("checked", styleDisabled); persistentOnly.setAttribute("checked", !altStyleSelected && !styleDisabled); persistentOnly.hidden = (window.content.document.preferredStylesheetSet && altSheetsAdded) ? true : false; sep.hidden = (noStyle.hidden && persistentOnly.hidden) || !haveAltSheets; return true; } ------------------ I've added the var "altSheetsAdded" that checks if we have added an additional menuitem for the preferred Stylesheet. The "Default Page Style"-menuitem may only be hidden, if this is true.

The additional variable in my patch may be not needed. It should be sufficient change the line from persistentOnly.hidden = (window.content.document.preferredStylesheetSet) ? true : false; to persistentOnly.hidden = (window.content.document.preferredStylesheetSet && haveAltSheets) ? true : false;

Better patch may be: persistentOnly.hidden = (window.content.document.preferredStylesheetSet) ? haveAltSheets : false;

Jonathan, if you attach your patch to this bug you can request a review, otherwise it will probably sit here unseen. http://www.mozilla.org/hacking/life-cycle.html http://www.mozilla.org/projects/firefox/review.html

(In reply to comment #3) > Jonathan, if you attach your patch to this bug you can request a review, > otherwise it will probably sit here unseen. > > http://www.mozilla.org/hacking/life-cycle.html > http://www.mozilla.org/projects/firefox/review.html You've forgotten the most important link from Fix Bugs at http://www.mozilla.org/contribute/ i.e. the link to Patch Maker: http://www.gerv.net/software/patch-maker/build-mode.html

Here is my patch. I don't know if this works properly, because it's my first patch an i had some problems with the Patch Maker.

Jonathan, you don't need to request "superreview?" for Firefox-only patches, just "review?". You also need to pick a reviewer, the Firefox reviewers are listed at the bottom of the following page. Pick one then click "Edit" next to your patch and enter your chosen reviewer's e-mail address into the requestee field: http://www.mozilla.org/projects/firefox/review.html

Comment on attachment 212521 [details] [diff] [review] Patch Thanks for your help. I hope this is better now.

I suppose that the path of the browser.js in my first patch is not correct. This patch should work better.

Is there a testcase for this bug that still works? It appears as though the site in the URL field has changed and can no longer be used to reproduce the bug.

Thanks for the testcase, Jonathan. The patch does look correct to me.

It would also be possible (and perhaps cleaner) to set the preferred stylesheet only, if it has media="screen" or media="all", but I do not know if this would confirm with the specs or if this is easy to implement.

Checked in on the trunk and the 1.8 branch for Firefox 2. mozilla/browser/base/content/browser.js 1.479.2.89 mozilla/browser/base/content/browser.js 1.585

i noticed this last year sometime, but didn't think to file a bug for it. glad it's fixed. :)