Use radio buttons for 'Alt text' in Image dialog, b=114531, r=brade, sr=kin

editor/ui/dialogs/content/EdImageOverlay.js

@@ -23,8 +23,19 @@
  *   Neil Rashbrook <neil@parkwaycc.co.uk> (Separated from EdImageProps.js)
  */
 
+/*
+ Note: We encourage non-empty alt text for images inserted into a page. 
+ When there's no alt text, we always write 'alt=""' as the attribute, since "alt" is a required attribute.
+ We allow users to not have alt text by checking a "Don't use alterate text" radio button,
+ and we don't accept spaces as valid alt text. A space used to be required to avoid the error message
+ if user didn't enter alt text, but is uneccessary now that we no longer annoy the user 
+ with the error dialog if alt="" is present on an img element.
+ We trim all spaces at the beginning and end of user's alt text
+*/
+
 var gInsertNewImage = true;
 var gInsertNewIMap = true;
+var gDoAltTextError = false;
 var gConstrainOn = false;
 // Note used in current version, but these are set correctly
 //  and could be used to reset width and height used for constrain ratio
@@ -35,8 +46,6 @@ var gImageMap = 0;
 var gCanRemoveImageMap = false;
 var gRemoveImageMap = false;
 var gImageMapDisabled = false;
-var firstTimeOkUsed = true;
-var doAltTextError = false;
 var actualWidth = "";
 var gOriginalSrc = "";
 var actualHeight = "";
@@ -59,6 +68,9 @@ function ImageStartup()
   gDialog.tabBorder         = document.getElementById( "imageBorderTab" );
   gDialog.srcInput          = document.getElementById( "srcInput" );
   gDialog.altTextInput      = document.getElementById( "altTextInput" );
+  gDialog.altTextRadioGroup = document.getElementById( "altTextRadioGroup" );
+  gDialog.altTextRadio      = document.getElementById( "altTextRadio" );
+  gDialog.noAltTextRadio    = document.getElementById( "noAltTextRadio" );
   gDialog.customSizeRadio   = document.getElementById( "customSizeRadio" );
   gDialog.actualSizeRadio   = document.getElementById( "actualSizeRadio" );
   gDialog.constrainCheckbox = document.getElementById( "constrainCheckbox" );
@@ -92,7 +104,20 @@ function InitImage()
   // Force loading of image from its source and show preview image
   LoadPreviewImage();
 
-  gDialog.altTextInput.value = globalElement.getAttribute("alt");
+  var hasAltText = globalElement.hasAttribute("alt");
+  if (hasAltText)
+    gDialog.altTextInput.value = globalElement.getAttribute("alt");
+
+  if (gInsertNewImage || !hasAltText || (hasAltText && gDialog.altTextInput.value))
+  {
+    SetAltTextDisabled(false);
+    gDialog.altTextRadioGroup.selectedItem = gDialog.altTextRadio;
+  }
+  else
+  {
+    SetAltTextDisabled(true);
+    gDialog.altTextRadioGroup.selectedItem = gDialog.noAltTextRadio;
+  }
 
   // setup the height and width widgets
   var width = InitPixelOrPercentMenulist(globalElement,
@@ -119,7 +144,7 @@ function InitImage()
   gDialog.imagetbInput.value = globalElement.getAttribute("vspace");
 
   // dialog.border.value       = globalElement.getAttribute("border");
-  bv = GetHTMLOrCSSStyleValue(globalElement, "border", "border-top-width");
+  var bv = GetHTMLOrCSSStyleValue(globalElement, "border", "border-top-width");
   var pxIndex = bv.search(/px/);
   if (pxIndex  > 0)
   {
@@ -142,11 +167,12 @@ function InitImage()
 
   // Get alignment setting
   var align = globalElement.getAttribute("align");
-  if (align) {
+  if (align)
     align = align.toLowerCase();
-  }
+
   var imgClass;
   var textID;
+
   switch ( align )
   {
     case "top":
@@ -166,6 +192,12 @@ function InitImage()
   doDimensionEnabling();
 }
 
+// Disable alt text input when "Don't use alt" radio is checked
+function SetAltTextDisabled(disable)
+{
+  gDialog.altTextInput.disabled = disable;
+}
+
 function GetImageMap()
 {
   var usemap = globalElement.getAttribute("usemap");
@@ -452,16 +484,18 @@ function ValidateImage()
   var src = TrimString(gDialog.srcInput.value);
   globalElement.setAttribute("src", src);
 
-  // Note: allow typing spaces,
-  // Warn user if empty string just once per dialog session
-  //   but don't consider this a failure
-  var alt = gDialog.altTextInput.value;
-  if (doAltTextError && !alt)
+  // Force user to enter Alt text only if "Alternate text" radio is checked
+  // Don't allow just spaces in alt text
+  var alt = "";
+  var useAlt = gDialog.altTextRadioGroup.selectedItem == gDialog.altTextRadio;
+  if (useAlt)
+    alt = TrimString(gDialog.altTextInput.value);
+
+  if (gDoAltTextError && useAlt && !alt)
   {
     AlertWithTitle(null, GetString("NoAltText"));
     SwitchToValidatePanel();
     gDialog.altTextInput.focus();
-    doAltTextError = false;
     return false;
   }
   globalElement.setAttribute("alt", alt);
@@ -543,12 +577,8 @@ function ValidateImage()
 
 function onAccept()
 {
-  // Show alt text error only once
-  // (we don't initialize doAltTextError=true
-  //  so Advanced edit button dialog doesn't trigger that error message)
-  doAltTextError = firstTimeOkUsed;
-  firstTimeOkUsed = false;
-
+  // Use this now (default = false) so Advanced Edit button dialog doesn't trigger error message
+  gDoAltTextError = true;
 
   if (ValidateData())
   {
@@ -623,5 +653,8 @@ function onAccept()
     SaveWindowLocation();
     return true;
   }
+
+  gDoAltTextError = false;
+
   return false;
 }

editor/ui/dialogs/content/EdImageOverlay.xul

@@ -38,14 +38,13 @@
   <!-- This tab is currently not used -->
   <tab id="imageLinkTab" label="&imageLinkTab.label;"/>
 
-  <groupbox id="imageLocation">
-    <caption label="&locationBox.label;"/>
-    <!--/////// Src URL and ALT Text //////-->
-    <label control   = "srcInput"
+  <vbox id="imageLocation"> 
+    <spacer class="spacer"/>
+    <label control = "srcInput"
       value = "&locationEditField.label;"
       tooltiptext="&locationEditField.tooltip;"
       />
-    <textbox
+    <textbox flex="1"
       id         = "srcInput" 
       oninput    = "ChangeImageSrc()"
       style      = "min-width : 20em"/>
@@ -58,18 +57,22 @@
       <button id="ChooseFile"/>
     </hbox>
     <spacer class="spacer"/>
-    <hbox>
-      <label id      = "altTextLabel" 
-        control     = "altTextInput"
-        tooltip = "aTooltip" tooltiptext="&altTextEditField.tooltip;"
-        value   = "&altTextEditField.label;"/>
-      <textbox
-        id     ="altTextInput"
-        style  = "min-width : 20em"
-        tooltip = "aTooltip" tooltiptext="&altTextEditField.tooltip;"/>
-    </hbox>
-    <spacer class="spacer"/>
-  </groupbox>
+    <radiogroup id="altTextRadioGroup" flex="1">
+      <hbox align="center">
+        <radio id = "altTextRadio" 
+          label   = "&altText.label;"
+          oncommand = "SetAltTextDisabled(false);"/>
+        <textbox flex="1"
+          id      = "altTextInput"
+          style   = "min-width : 20em"
+          tooltip = "aTooltip" tooltiptext="&altTextEditField.tooltip;"
+          oninput = "SetAltTextDisabled(false);"/>
+      </hbox>
+      <radio id = "noAltTextRadio" 
+        label   = "&noAltText.label;"
+        oncommand = "SetAltTextDisabled(true);"/>
+    </radiogroup>
+  </vbox>
 
   <groupbox id="imagePreview" orient="horizontal">
     <caption label="&previewBox.label;"/>
@@ -95,150 +98,147 @@
     </vbox>
   </groupbox>
 
-  <groupbox id="imageDimensions" align="start">
-    <caption id="dimensionsLabel" label="&dimensionsBox.label;"/>
-    <vbox>
-          <hbox>
-            <radiogroup id="imgSizeGroup">
-              <radio
-                id      = "actualSizeRadio" 
-                label   = "&actualSizeRadio.label;"
-                tooltip = "aTooltip" tooltiptext="&actualSizeRadio.tooltip;"
-                oncommand = "SetActualSize()"/>
-              <radio
-                id      = "customSizeRadio" 
-                label   = "&customSizeRadio.label;"
-                tooltip = "aTooltip" tooltiptext="&customSizeRadio.tooltip;"
-                oncommand = "doDimensionEnabling();" />
-            </radiogroup>
-            <spacer flex="1"/>
-            <vbox>
-              <spacer flex="1"/>
-              <!--////// CONSTRAIN DIMENSIONS //////-->
-              <checkbox id="constrainCheckbox" label="&constrainCheckbox.label;"
-                   oncommand="ToggleConstrain()"
-                   tooltiptext="&constrainCheckbox.tooltip;"/>
-            </vbox>
-            <spacer flex="1"/>
-          </hbox>
-          <spacer class="spacer"/>
-          <grid class="big-left-margin">
-            <columns><column/><column/><column flex="1"/></columns>
-            <rows>
-              <!--//////  IMAGE WIDTH  //////-->
-              <row align="center">
-                <label id    = "widthLabel"
-                  control   = "widthInput"
-                  value = "&widthEditField.label;" />
-                <textbox
-                  id       = "widthInput" 
-                  class    = "narrow"
-                  oninput  = "constrainProportions(this.id, 'heightInput')"/>
-                <menulist id = "widthUnitsMenulist"
-                  oncommand = "doDimensionEnabling();" />
-                <!-- contents are appended by JS -->
-              </row>
-              <!--////// IMAGE HEIGHT //////-->
-              <row align="center">
-                <label id    = "heightLabel"
-                  control   = "heightInput"
-                  value = "&heightEditField.label;" />
-                <textbox
-                  id       = "heightInput" 
-                  class    = "narrow"
-                  oninput  = "constrainProportions(this.id, 'widthInput')"/>
-                <menulist id = "heightUnitsMenulist"
-                  oncommand = "doDimensionEnabling();" />
-                <!-- contents are appended by JS -->
-              </row>
-            </rows>          
-          </grid>
-          <spacer flex="1"/>
-    </vbox>
-  </groupbox>
+  <vbox id="imageDimensions" align="start">
+    <spacer class="spacer"/>
+    <hbox>
+      <radiogroup id="imgSizeGroup">
+        <radio
+          id      = "actualSizeRadio" 
+          label   = "&actualSizeRadio.label;"
+          tooltip = "aTooltip" tooltiptext="&actualSizeRadio.tooltip;"
+          oncommand = "SetActualSize()"/>
+        <radio
+          id      = "customSizeRadio" 
+          label   = "&customSizeRadio.label;"
+          tooltip = "aTooltip" tooltiptext="&customSizeRadio.tooltip;"
+          oncommand = "doDimensionEnabling();" />
+      </radiogroup>
+      <spacer flex="1"/>
+      <vbox>
+        <spacer flex="1"/>
+        <checkbox id="constrainCheckbox" label="&constrainCheckbox.label;"
+             oncommand="ToggleConstrain()"
+             tooltiptext="&constrainCheckbox.tooltip;"/>
+      </vbox>
+      <spacer flex="1"/>
+    </hbox>
+    <spacer class="spacer"/>
+    <grid class="big-left-margin">
+      <columns><column/><column/><column flex="1"/></columns>
+      <rows>
+        <row align="center">
+          <label id    = "widthLabel"
+            control   = "widthInput"
+            value = "&widthEditField.label;" />
+          <textbox
+            id       = "widthInput" 
+            class    = "narrow"
+            oninput  = "constrainProportions(this.id, 'heightInput')"/>
+          <menulist id = "widthUnitsMenulist"
+            oncommand = "doDimensionEnabling();" />
+          <!-- contents are appended by JS -->
+        </row>
+        <row align="center">
+          <label id    = "heightLabel"
+            control   = "heightInput"
+            value = "&heightEditField.label;" />
+          <textbox
+            id       = "heightInput" 
+            class    = "narrow"
+            oninput  = "constrainProportions(this.id, 'widthInput')"/>
+          <menulist id = "heightUnitsMenulist"
+            oncommand = "doDimensionEnabling();" />
+          <!-- contents are appended by JS -->
+        </row>
+      </rows>          
+    </grid>
+    <spacer flex="1"/>
+  </vbox>
 
   <hbox id="imageAppearance">
     <groupbox>
-          <caption id="spacingLabel" label="&spacingBox.label;"/>
-          <grid>
-            <columns><column/><column/><column/></columns>
-            <rows>
-              <row align="center">
-                <label 
-                  class = "align-right" 
-                  id    = "leftrightLabel"
-                  value = "&leftRightEditField.label;"/>
-                <textbox
-                  class     = "narrow"
-                  id        = "imageleftrightInput" 
-                  oninput   = "forceInteger(this.id)"/>
-                <label 
-                  id    = "leftrighttypeLabel"
-                  value = "&pixelsPopup.value;" />
-              </row>
-              <row align="center">
-                <label 
-                  class = "align-right" 
-                  id    = "topbottomLabel"
-                  value = "&topBottomEditField.label;"/>
-                <textbox
-                  class     = "narrow"
-                  id        = "imagetopbottomInput" 
-                  oninput   = "forceInteger(this.id)"/>
-                <label
-                  id    = "topbottomtypeLabel"
-                  value = "&pixelsPopup.value;" />
-              </row>
-              <row align="center">
-                <label class = "align-right" 
-                  id    = "borderLabel"
-                  value = "&borderEditField.label;"/>
-                <textbox
-                  class     = "narrow"
-                  id        = "border" 
-                  oninput   = "forceInteger(this.id)"/>
-                <label 
-                  id    = "bordertypeLabel"
-                  value = "&pixelsPopup.value;" />
-              </row>
-            </rows>
-          </grid>
-        </groupbox>
+      <caption id="spacingLabel" label="&spacingBox.label;"/>
+      <grid>
+        <columns><column/><column/><column/></columns>
+        <rows>
+          <row align="center">
+            <label 
+              class = "align-right" 
+              id    = "leftrightLabel"
+              value = "&leftRightEditField.label;"/>
+            <textbox
+              class     = "narrow"
+              id        = "imageleftrightInput" 
+              oninput   = "forceInteger(this.id)"/>
+            <label 
+              id    = "leftrighttypeLabel"
+              value = "&pixelsPopup.value;" />
+          </row>
+          <spacer class="spacer"/>
+          <row align="center">
+            <label 
+              class = "align-right" 
+              id    = "topbottomLabel"
+              value = "&topBottomEditField.label;"/>
+            <textbox
+              class     = "narrow"
+              id        = "imagetopbottomInput" 
+              oninput   = "forceInteger(this.id)"/>
+            <label
+              id    = "topbottomtypeLabel"
+              value = "&pixelsPopup.value;" />
+          </row>
+          <spacer class="spacer"/>
+          <row align="center">
+            <label class = "align-right" 
+              id    = "borderLabel"
+              value = "&borderEditField.label;"/>
+            <textbox
+              class     = "narrow"
+              id        = "border" 
+              oninput   = "forceInteger(this.id)"/>
+            <label 
+              id    = "bordertypeLabel"
+              value = "&pixelsPopup.value;" />
+          </row>
+        </rows>
+      </grid>
+    </groupbox>
 
     <vbox>
       <groupbox align="start">
-          <caption id="alignLabel" label="&alignment.label;"/>
-          <menulist id="alignTypeSelect" class="align-menu">
-            <menupopup>
-              <menuitem class="align-menu" value="top"    label="&topPopup.value;"/>
-              <menuitem class="align-menu" value="middle" label="&centerPopup.value;"/>
-              <menuitem class="align-menu" value="bottom" label="&bottomPopup.value;"/>
-              <!-- HTML attribute value is opposite of the button label on purpose -->
-              <menuitem class="align-menu" value="right"  label="&wrapLeftPopup.value;"/>
-              <menuitem class="align-menu" value="left"   label="&wrapRightPopup.value;"/>
-            </menupopup>
-          </menulist>
-        </groupbox>
+        <caption id="alignLabel" label="&alignment.label;"/>
+        <menulist id="alignTypeSelect" class="align-menu">
+          <menupopup>
+            <menuitem class="align-menu" value="top"    label="&topPopup.value;"/>
+            <menuitem class="align-menu" value="middle" label="&centerPopup.value;"/>
+            <menuitem class="align-menu" value="bottom" label="&bottomPopup.value;"/>
+            <!-- HTML attribute value is opposite of the button label on purpose -->
+            <menuitem class="align-menu" value="right"  label="&wrapLeftPopup.value;"/>
+            <menuitem class="align-menu" value="left"   label="&wrapRightPopup.value;"/>
+          </menupopup>
+        </menulist>
+      </groupbox>
 
       <groupbox>
-         <caption id="imagemapLabel" label="&imagemapBox.label;"/>
-         <hbox equalsize="always">
+        <caption id="imagemapLabel" label="&imagemapBox.label;"/>
+        <hbox equalsize="always">
 
 <!-- Hide Image Map Editor. Not ready for prime time yet
-          <button
-            id      = "editImageMap"
-            oncommand = "editImageMap()"
-            tooltiptext="&editImageMapButton.tooltip;"
-            label   = "&editImageMapButton.label;"
-            flex    = "1"/>
+        <button
+          id      = "editImageMap"
+          oncommand = "editImageMap()"
+          tooltiptext="&editImageMapButton.tooltip;"
+          label   = "&editImageMapButton.label;"
+          flex    = "1"/>
 -->
-          <button
-            id      = "removeImageMap"
-            oncommand = "removeImageMap()"
-            label   = "&removeImageMapButton.label;"
-            flex    = "1"/>
-          <spacer flex="1"/><!-- remove when we restore Image Map Editor -->
-          </hbox>
+        <button
+          id      = "removeImageMap"
+          oncommand = "removeImageMap()"
+          label   = "&removeImageMapButton.label;"
+          flex    = "1"/>
+        <spacer flex="1"/><!-- remove when we restore Image Map Editor -->
+        </hbox>
       </groupbox>
     </vbox>
   </hbox>

editor/ui/dialogs/locale/en-US/EditorImageProperties.dtd

@@ -20,7 +20,7 @@
    - Contributor(s): 
   -->
 
-<!-- These strings are for use specifically in the editor's image dialog. -->
+<!-- These strings are for use specifically in the editor's image and form image dialogs. -->
 
 <!-- Window title -->
 <!ENTITY windowTitle.label "Image Properties">
@@ -28,18 +28,17 @@
 <!ENTITY pixelsPopup.value "pixels">
 <!ENTITY percentPopup.value "percent">
 
-<!-- These are in the location box. -->
-<!ENTITY locationBox.label "Image Information">
-<!ENTITY locationEditField.label "Image URL:">
+<!-- These are in the Location tab panel -->
+<!ENTITY locationEditField.label "Image Location:">
 <!ENTITY locationEditField.tooltip "Type the image's filename or location">
-<!ENTITY altTextEditField.label "Alternative Text:">
+<!ENTITY altText.label "Alternate text:">
 <!ENTITY altTextEditField.tooltip "Type text to display in place of the image">
+<!ENTITY noAltText.label "Don't use alternate text">
 
 <!ENTITY previewBox.label "Image Preview">
-
 <!ENTITY MoreFewerButton.tooltip "Display more or fewer properties to edit">
-<!-- These controls are in the Dimensions box of the expanded area -->
-<!ENTITY dimensionsBox.label "Dimensions">
+
+<!-- These controls are in the Dimensions tab panel -->
 <!-- actualSize.label should be same as actualSizeRadio.label + ":" -->
 <!ENTITY actualSize.label "Actual Size:">
 <!ENTITY actualSizeRadio.label "Actual Size">
@@ -65,7 +64,7 @@
 <!ENTITY wrapRightPopup.value "Wrap to the right">
 <!ENTITY wrapLeftPopup.value "Wrap to the left">
 
-<!-- These controls are in the Spacing Box of the expanded area -->
+<!-- These controls are in the Spacing Box -->
 <!ENTITY spacingBox.label "Spacing">
 <!ENTITY spacingBox.tooltip "Type a number for the amount of space around image">
 <!ENTITY leftRightEditField.label "Left and Right:">
@@ -80,7 +79,7 @@
 
 <!-- These tabs are currently used in the image input dialog -->
 <!ENTITY imageInputTab.label "Form">
-<!ENTITY imageLocationTab.label "Image">
+<!ENTITY imageLocationTab.label "Location">
 <!ENTITY imageDimensionsTab.label "Dimensions">
 <!ENTITY imageAppearanceTab.label "Appearance">
 <!-- This tab is currently not used -->