Bug 849438 - Searches in ListControlFrames should ignore  . r=bz
☠☠ backed out by 7731f5947789 ☠ ☠
authorMounir Lamouri <mounir.lamouri@gmail.com>
Tue, 19 Mar 2013 18:24:35 +0000
changeset 131908 2b2de9cc2f59ebd402b2eda8168e92c4c19e4f1c
parent 131907 26b4880c8a64c5cf49ea1d1ae06242168d86a6ba
child 131909 a382fbb818ad27a3368fdb98544d2a77e298f502
push idunknown
push userunknown
push dateunknown
reviewersbz
bugs849438
milestone22.0a1
Bug 849438 - Searches in ListControlFrames should ignore &nbsp;. r=bz
content/base/public/nsContentUtils.h
content/base/src/nsContentUtils.cpp
layout/forms/nsListControlFrame.cpp
layout/forms/test/Makefile.in
layout/forms/test/test_listcontrol_search.html
--- a/content/base/public/nsContentUtils.h
+++ b/content/base/public/nsContentUtils.h
@@ -329,16 +329,22 @@ public:
    *
    * We define whitespace using the list in HTML5 and css3-selectors:
    * U+0009, U+000A, U+000C, U+000D, U+0020
    *
    * HTML 4.01 also lists U+200B (zero-width space).
    */
   static bool IsHTMLWhitespace(PRUnichar aChar);
 
+  /*
+   * Returns whether the character is an HTML whitespace (see IsHTMLWhitespace)
+   * or a nbsp character (U+00A0).
+   */
+  static bool IsHTMLWhitespaceOrNBSP(PRUnichar aChar);
+
   /**
    * Is the HTML local name a block element?
    */
   static bool IsHTMLBlock(nsIAtom* aLocalName);
 
   /**
    * Is the HTML local name a void element?
    */
--- a/content/base/src/nsContentUtils.cpp
+++ b/content/base/src/nsContentUtils.cpp
@@ -1196,16 +1196,23 @@ nsContentUtils::IsHTMLWhitespace(PRUnich
          aChar == PRUnichar(0x000A) ||
          aChar == PRUnichar(0x000C) ||
          aChar == PRUnichar(0x000D) ||
          aChar == PRUnichar(0x0020);
 }
 
 /* static */
 bool
+nsContentUtils::IsHTMLWhitespaceOrNBSP(PRUnichar aChar)
+{
+  return IsHTMLWhitespace(aChar) || aChar == PRUnichar(0xA0);
+}
+
+/* static */
+bool
 nsContentUtils::IsHTMLBlock(nsIAtom* aLocalName)
 {
   return
     (aLocalName == nsGkAtoms::address) ||
     (aLocalName == nsGkAtoms::article) ||
     (aLocalName == nsGkAtoms::aside) ||
     (aLocalName == nsGkAtoms::blockquote) ||
     (aLocalName == nsGkAtoms::center) ||
@@ -2153,16 +2160,19 @@ nsContentUtils::TrimWhitespace(const nsA
 // inlining the method. Considering there is not so much spaces checking
 // methods we can consider this to be better than inlining.
 template
 const nsDependentSubstring
 nsContentUtils::TrimWhitespace<nsCRT::IsAsciiSpace>(const nsAString&, bool);
 template
 const nsDependentSubstring
 nsContentUtils::TrimWhitespace<nsContentUtils::IsHTMLWhitespace>(const nsAString&, bool);
+template
+const nsDependentSubstring
+nsContentUtils::TrimWhitespace<nsContentUtils::IsHTMLWhitespaceOrNBSP>(const nsAString&, bool);
 
 static inline void KeyAppendSep(nsACString& aKey)
 {
   if (!aKey.IsEmpty()) {
     aKey.Append('>');
   }
 }
 
--- a/layout/forms/nsListControlFrame.cpp
+++ b/layout/forms/nsListControlFrame.cpp
@@ -2467,38 +2467,42 @@ nsListControlFrame::KeyPress(nsIDOMEvent
       // *at* 0.
       int32_t startIndex = GetSelectedIndex();
       if (startIndex == kNothingSelected) {
         startIndex = 0;
       } else if (stringLength == 1) {
         startIndex++;
       }
 
-      uint32_t i;
-      for (i = 0; i < numOptions; i++) {
+      for (uint32_t i = 0; i < numOptions; ++i) {
         uint32_t index = (i + startIndex) % numOptions;
-        nsCOMPtr<nsIDOMHTMLOptionElement> optionElement =
-          GetOption(options, index);
-        if (optionElement) {
-          nsAutoString text;
-          if (NS_OK == optionElement->GetText(text)) {
-            if (StringBeginsWith(text, incrementalString,
-                                 nsCaseInsensitiveStringComparator())) {
-              bool wasChanged = PerformSelection(index, isShift, isControl);
-              if (wasChanged) {
-                // dispatch event, update combobox, etc.
-                if (!UpdateSelection()) {
-                  return NS_OK;
-                }
-              }
-              break;
-            }
-          }
+        nsCOMPtr<nsIDOMHTMLOptionElement> optionElement = GetOption(options, index);
+        if (!optionElement) {
+          continue;
+        }
+
+        nsAutoString text;
+        if (NS_FAILED(optionElement->GetText(text)) ||
+            !StringBeginsWith(nsContentUtils::TrimWhitespace<nsContentUtils::IsHTMLWhitespaceOrNBSP>(text, false),
+                              incrementalString,
+                              nsCaseInsensitiveStringComparator())) {
+          continue;
         }
-      } // for
+
+        if (!PerformSelection(index, isShift, isControl)) {
+          break;
+        }
+
+        // If UpdateSelection() returns false, that means the frame is no longer
+        // alive. We should stop doing anything.
+        if (!UpdateSelection()) {
+          return NS_OK;
+        }
+        break;
+      }
 
     } break;//case
   } // switch
 
   // We ate the key if we got this far.
   aKeyEvent->PreventDefault();
 
   // If we didn't do an incremental search, clear the string
--- a/layout/forms/test/Makefile.in
+++ b/layout/forms/test/Makefile.in
@@ -39,16 +39,17 @@ MOCHITEST_FILES =	test_bug231389.html \
 		test_bug572406.html \
 		test_bug572649.html \
 		test_bug36619.html \
 		test_bug620936.html \
 		test_bug595310.html \
 		test_bug644542.html \
 		test_bug672810.html \
 		test_bug704049.html \
+		test_listcontrol_search.html \
 		$(NULL)
 
 MOCHITEST_CHROME_FILES = \
 		     bug536567_iframe.html \
 		test_bug536567_perwindowpb.html \
 		     bug536567_subframe.html \
 		test_bug665540.html \
 		     bug665540_window.xul \
new file mode 100644
--- /dev/null
+++ b/layout/forms/test/test_listcontrol_search.html
@@ -0,0 +1,46 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=849438
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for &lt;select&gt; list control search</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <script type="application/javascript">
+
+  /** This test will focus a select element and press a key that matches the
+      first non-space character of an entry. **/
+
+  SimpleTest.waitForExplicitFinish();
+  SimpleTest.waitForFocus(function() {
+    var select = document.getElementsByTagName('select')[0];
+    select.focus();
+    synthesizeKey('a', {});
+
+    is(select.options[0].selected, false, "the first option isn't selected");
+    is(select.options[1].selected, true, "the second option is selected");
+
+    select.blur();
+
+    SimpleTest.finish();
+  });
+
+  </script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=849438">Mozilla Bug 849438</a>
+<p id="display"></p>
+<div id="content">
+  <select>
+    <option>Please select an entry</option>
+    <option>&nbsp;a</option>
+    <option>&nbsp;b</option>
+  </select>
+</div>
+<pre id="test">
+</pre>
+</body>
+</html>