Per comments in bug 831955, we need to make sure our flyouts use one of two widths - narrow (346 pixels) or wide (646 pixels).

If we do this and it causes any extra work, we should backout instead of posting new defects for v1.

p=1

I will give this a shot!

Changed the widths for both prefs & about panels to 346 as per comment 0. Ally, looks like you have fixed the width for the sync panel in bug 845468. Made sure that all the panel widths matched in Firefox Metro, also ensured that nothing was being cut off for "Options", "Sync" & "About". Went through some basic functionality and ensured everything was still working correctly.

Comment on attachment 777545 [details] [diff] [review] fixed the widths for prefs/about panels Review of attachment 777545 [details] [diff] [review]: ----------------------------------------------------------------- Thanks! This patch is correct as is, but I think we can make it even better. Since we want all the flyouts to be the same width (and probably some other attributes), why don't we seet a class in the xul on the flyouts so we can use css class selectors? That way we can control the style of the flyouts in one place and ensure they always match. For an example, the "meta-section" class used here http://mxr.mozilla.org/mozilla-central/source/browser/metro/base/content/browser.xul#206 (you can see it used several places in the start page here) matches to http://mxr.mozilla.org/mozilla-central/source/browser/metro/theme/platform.css#657 So in browser.xul change the flyouts like <flyoutpanel id="prefs-flyoutpanel" headertext="&optionsHeader.title;"> to add something like <flyoutpanel id="prefs-flyoutpanel" class="prefFlyouts" headertext="&optionsHeader.title;"> add something like to browser.css .prefsFlyouts { // common rules in here like the width } Also please remove the old rules you've just consolidated from their id selectors[2] (like #about-flyoutpanel) as those have higher precedence [3] so they would override your new class rule. :) Please let us know if you have questions or get stuck. We're happy to help. [1] https://developer.mozilla.org/en-US/docs/Web/CSS/Class_selectors [2] https://developer.mozilla.org/en-US/docs/Web/Guide/CSS/Getting_started/Selectors [3] "CSS gives priority to the rule that has the more specific selector. An ID selector is more specific than a class selector, which in turn is more specific than a tag selector." from [2]

drive-by nit: Don't call the class "prefFlyouts" since that would be specific to just the "preferences" flyout. Something like "flyout" or "flyoutPanel" would be more appropriate. Also, it might be best to put the CSS for flyouts in flyoutpanel.css (maybe we can rename that to just "flyout.css"... but I digress)

Good idea re the class. Maybe call it narrowFlyout and we can later use wideFlyout if we need a wider one.

Thanks for the feedback! Will give it another shot tomorrow.

Created the narrowFlyout class and then added them into the Pref, Sync and About panels. Went through all three flyouts and made sure everything was still working, the correct widths are being used and nothing is being cut off. Thanks for the help and review Ally!

Comment on attachment 778781 [details] [diff] [review] fixed the widths for Prefs/Sync/About panels v2 Review of attachment 778781 [details] [diff] [review]: ----------------------------------------------------------------- Almost there! Looks good & mbrubeck was nice enough to test it while my trees build this morning. Tim is right that the new css rule should move to the flyoutpanels.css file. While the old rules did live in browser.css, now that we have a separate css file dedicated to flyouts, we should put all our css related to flyouts in there. The good news is that it should be a quick & easy change.

Sound good! I will change this on Friday when I get home from my work trip (currently traveling and don't have access to my environment PC as its in Toronto!

Comment on attachment 778781 [details] [diff] [review] fixed the widths for Prefs/Sync/About panels v2 look forward to seeing it. :)

Can one of you all take a look at bug 898457 and see if that's related or could be fixed here?

Looks like an unrelated issue, I think it's due to localized strings not wrapping in the about panel. Should be looked at independently to this bug.

Ally, added the following changes: - removed the class from browser.css and added it into flyoutpanels.css - renamed the class to flyout-narrow to match the naming convention in flyoutpanels.css - changed the names of the class in browser.xul to match the new name in flyoutpanels.css Went through all three flyout panels (Pref, About, Sync) and made sure they where working without any issues and used the same widths. Hopefully everything looks good this time! If not, just let me know if anything else needs to be changed.

Comment on attachment 782395 [details] [diff] [review] fixed the widths for Prefs/Sync/About panels v3 Review of attachment 782395 [details] [diff] [review]: ----------------------------------------------------------------- \o/ Let's Ship it! How about bug 885383 next? It's a little meatier.

Comment on attachment 784057 [details] Screenshot of the bug Whoops. I posted this screenshot in the wrong bug. Apologies.

Mozilla/5.0 (Windows NT 6.2; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0 Build ID: 20130819030205 WFM Tested on windows 8 64bit using latest Nightly for iteration #12. Flyouts seem to all have same widths. This should also be verified on a device.

User Agent: Mozilla/5.0 (Windows NT 6.2; WOW64; rv:26.0) Gecko/20100101 Firefox/26.0 Build ID: 20130826074752 Built from http://hg.mozilla.org/mozilla-central/rev/14b1e8c2957e WFM Tested on windows 8 using latest nightly for iteration-12. All fly-out width was same.