From ea7cea0305186bbf02f8048362e65ae066abb6a4 Mon Sep 17 00:00:00 2001 From: Geoff Lankow Date: Wed, 7 Mar 2012 00:41:25 +1300 Subject: [PATCH] Bug 731041 - Tidy up addon inline preferences code, toolkit; r=mbrubeck, Unfocused --- .../mozapps/extensions/content/extensions.js | 24 +--- .../mozapps/extensions/content/setting.xml | 124 ++++++++---------- .../browser_inlinesettings1/options.xul | 2 +- .../test/browser/browser_inlinesettings.js | 28 +--- .../mozapps/extensions/extensions.css | 16 ++- .../mozapps/extensions/extensions.css | 14 +- .../mozapps/extensions/extensions.css | 14 +- 7 files changed, 99 insertions(+), 123 deletions(-) diff --git a/toolkit/mozapps/extensions/content/extensions.js b/toolkit/mozapps/extensions/content/extensions.js index ff57058f874..3c83b4b9174 100644 --- a/toolkit/mozapps/extensions/content/extensions.js +++ b/toolkit/mozapps/extensions/content/extensions.js @@ -2882,12 +2882,9 @@ var gDetailView = { for (var i = 0; i < settings.length; i++) { var setting = settings[i]; - // Remove setting description, for replacement later var desc = stripTextNodes(setting).trim(); - if (setting.hasAttribute("desc")) { - desc = setting.getAttribute("desc").trim(); - setting.removeAttribute("desc"); - } + if (!setting.hasAttribute("desc")) + setting.setAttribute("desc", desc); var type = setting.getAttribute("type"); if (type == "file" || type == "directory") @@ -2899,23 +2896,10 @@ var gDetailView = { setting.setAttribute("first-row", true); firstSetting = setting; } - - // Add a new row containing the description - if (desc) { - var row = document.createElement("row"); - if (!visible) { - row.setAttribute("unsupported", "true"); - } - var label = document.createElement("label"); - label.className = "preferences-description"; - label.textContent = desc; - row.appendChild(label); - rows.appendChild(row); - } } - // Ensure the page has loaded and force the XBL bindings to be synchronously applied, - // then notify observers. + // Ensure the page has loaded and force the XBL bindings to be synchronously applied, + // then notify observers. if (gViewController.viewPort.selectedPanel.hasAttribute("loading")) { gDetailView.node.addEventListener("ViewChanged", function viewChangedEventListener() { gDetailView.node.removeEventListener("ViewChanged", viewChangedEventListener, false); diff --git a/toolkit/mozapps/extensions/content/setting.xml b/toolkit/mozapps/extensions/content/setting.xml index 2c19075c322..a0af4a601e8 100644 --- a/toolkit/mozapps/extensions/content/setting.xml +++ b/toolkit/mozapps/extensions/content/setting.xml @@ -51,7 +51,7 @@ - + @@ -160,7 +160,6 @@ false document.getAnonymousElementByAttribute(this, "anonid", "input"); - this.parentNode.localName == "settings" ? this.parentNode : null; @@ -169,14 +168,14 @@ - - - - - + + + + + - - + + @@ -204,19 +203,7 @@ - - - - - - - - - - - - - + @@ -234,9 +221,6 @@ ]]> - - - @@ -263,20 +247,20 @@ ]]> - - - - - - + + + + + - - + + @@ -297,21 +281,18 @@ ]]> - - - - - - - - + + + + + - + @@ -319,14 +300,15 @@ - - - - - + + + + + - - + + @@ -350,20 +332,18 @@ ]]> - - - - - - - + + + + + - + @@ -394,15 +374,15 @@ - - - - - + + + + + - - - + + + @@ -476,14 +456,14 @@ - - - - - + + + + + - - + + diff --git a/toolkit/mozapps/extensions/test/browser/addons/browser_inlinesettings1/options.xul b/toolkit/mozapps/extensions/test/browser/addons/browser_inlinesettings1/options.xul index febf7bbc8a0..095d3bcef20 100644 --- a/toolkit/mozapps/extensions/test/browser/addons/browser_inlinesettings1/options.xul +++ b/toolkit/mozapps/extensions/test/browser/addons/browser_inlinesettings1/options.xul @@ -1,6 +1,6 @@ - + diff --git a/toolkit/mozapps/extensions/test/browser/browser_inlinesettings.js b/toolkit/mozapps/extensions/test/browser/browser_inlinesettings.js index 1e331027b15..849c11cc7c1 100644 --- a/toolkit/mozapps/extensions/test/browser/browser_inlinesettings.js +++ b/toolkit/mozapps/extensions/test/browser/browser_inlinesettings.js @@ -21,7 +21,7 @@ var observer = { // Test if the binding has applied before the observers are notified. We test the second setting here, // because the code operates on the first setting and we want to check it applies to all. var setting = aSubject.querySelector("rows > setting[first-row] ~ setting"); - var input = gManagerWindow.document.getAnonymousElementByAttribute(setting, "class", "setting-label"); + var input = gManagerWindow.document.getAnonymousElementByAttribute(setting, "class", "preferences-title"); isnot(input, null, "XBL binding should be applied"); // Add some extra height to the scrolling pane to ensure that it needs to scroll when appropriate. @@ -174,6 +174,7 @@ add_test(function() { Services.prefs.setBoolPref("extensions.inlinesettings1.bool", false); var input = gManagerWindow.document.getAnonymousElementByAttribute(settings[0], "anonid", "input"); isnot(input.checked, true, "Checkbox should have initial value"); + is(input.label, "Check box label", "Checkbox should be labelled"); EventUtils.synthesizeMouseAtCenter(input, { clickCount: 1 }, gManagerWindow); is(input.checked, true, "Checkbox should have updated value"); is(Services.prefs.getBoolPref("extensions.inlinesettings1.bool"), true, "Bool pref should have been updated"); @@ -381,45 +382,26 @@ add_test(function() { is(node.nodeName, "setting", "Should be a setting node"); ok(node.hasAttribute("first-row"), "First visible row should have first-row attribute"); var description = gManagerWindow.document.getAnonymousElementByAttribute(node, "class", "preferences-description"); - is(description.textContent.trim(), "", "Description node should be empty"); - - node = node.nextSibling; - is(node.nodeName, "row", "Setting should be followed by a row node"); - is_element_visible(node, "Description should be visible"); - is(node.textContent, "Description Attribute", "Description should be in this row"); + is(description.textContent, "Description Attribute", "Description node should contain description"); node = settings[2]; is(node.nodeName, "setting", "Should be a setting node"); ok(!node.hasAttribute("first-row"), "Not the first row"); description = gManagerWindow.document.getAnonymousElementByAttribute(node, "class", "preferences-description"); - is(description.textContent.trim(), "", "Description node should be empty"); - - node = node.nextSibling; - is(node.nodeName, "row", "Setting should be followed by a row node"); - is_element_visible(node, "Description should be visible"); - is(node.textContent, "Description Text Node", "Description should be in this row"); + is(description.textContent, "Description Text Node", "Description node should contain description"); node = settings[3]; is(node.nodeName, "setting", "Should be a setting node"); ok(!node.hasAttribute("first-row"), "Not the first row"); description = gManagerWindow.document.getAnonymousElementByAttribute(node, "class", "preferences-description"); - is(description.textContent.trim(), "", "Description node should be empty"); + is(description.textContent, "This is a test, all this text should be visible", "Description node should contain description"); var button = node.firstElementChild; isnot(button, null, "There should be a button"); - node = node.nextSibling; - is(node.nodeName, "row", "Setting should be followed by a row node"); - is_element_visible(node, "Description should be visible"); - is(node.textContent.trim(), "This is a test, all this text should be visible", "Description should be in this row"); - node = settings[4]; is_element_hidden(node, "Unsupported settings should not be visible"); ok(!node.hasAttribute("first-row"), "Hidden row is not the first row"); - node = node.nextSibling; - is(node.nodeName, "row", "Setting should be followed by a row node"); - is_element_hidden(node, "Descriptions of unsupported settings should not be visible"); - var button = gManagerWindow.document.getElementById("detail-prefs-btn"); is_element_hidden(button, "Preferences button should not be visible"); diff --git a/toolkit/themes/gnomestripe/mozapps/extensions/extensions.css b/toolkit/themes/gnomestripe/mozapps/extensions/extensions.css index 385c9146254..dff21503981 100644 --- a/toolkit/themes/gnomestripe/mozapps/extensions/extensions.css +++ b/toolkit/themes/gnomestripe/mozapps/extensions/extensions.css @@ -739,7 +739,7 @@ setting[first-row="true"] { setting { border-top: 1px solid ThreeDShadow; -moz-box-align: center; - min-height: 33px; + min-height: 32px; } #detail-controls { @@ -755,15 +755,25 @@ setting[first-row="true"] { margin-top: 2em; } +setting { + -moz-box-align: start; +} + +.preferences-alignment { + min-height: 32px; + -moz-box-align: center; +} + .preferences-description { font-size: 90.9%; color: graytext; margin-top: -2px; -moz-margin-start: 2em; + white-space: pre-wrap; } -setting[type="string"] > .setting-input > textbox { - -moz-box-flex: 1; +.preferences-description:empty { + display: none; } menulist { /* Fixes some styling inconsistencies */ diff --git a/toolkit/themes/pinstripe/mozapps/extensions/extensions.css b/toolkit/themes/pinstripe/mozapps/extensions/extensions.css index 93f8775bb82..095dfc2bfe8 100644 --- a/toolkit/themes/pinstripe/mozapps/extensions/extensions.css +++ b/toolkit/themes/pinstripe/mozapps/extensions/extensions.css @@ -934,15 +934,25 @@ setting[first-row="true"] { margin-top: 2em; } +setting { + -moz-box-align: start; +} + +.preferences-alignment { + min-height: 30px; + -moz-box-align: center; +} + .preferences-description { font-size: 90.9%; color: graytext; margin-top: -2px; -moz-margin-start: 2em; + white-space: pre-wrap; } -setting[type="string"] > .setting-input > textbox { - -moz-box-flex: 1; +.preferences-description:empty { + display: none; } setting[type="radio"] > radiogroup { diff --git a/toolkit/themes/winstripe/mozapps/extensions/extensions.css b/toolkit/themes/winstripe/mozapps/extensions/extensions.css index c5e4156ab50..0c32802e062 100644 --- a/toolkit/themes/winstripe/mozapps/extensions/extensions.css +++ b/toolkit/themes/winstripe/mozapps/extensions/extensions.css @@ -913,15 +913,25 @@ setting[first-row="true"] { margin-top: 2em; } +setting { + -moz-box-align: start; +} + +.preferences-alignment { + min-height: 30px; + -moz-box-align: center; +} + .preferences-description { font-size: 90.9%; color: graytext; margin-top: -2px; -moz-margin-start: 2em; + white-space: pre-wrap; } -setting[type="string"] > .setting-input > textbox { - -moz-box-flex: 1; +.preferences-description:empty { + display: none; } setting[type="radio"] > radiogroup {