Attachment 46726 Details for Bug 33751 – Patch for platform-agnostic and Mac versions of the relevant code.

[patch] Patch for platform-agnostic and Mac versions of the relevant code.

7538330_patch.txt (text/plain), 6.58 KB, created by John Sullivan on 2010-01-15 19:10:39 PST

(hide)

Description:

Filename:

MIME Type:

Creator: John Sullivan

Created: 2010-01-15 19:10:39 PST

Size: 6.58 KB

patch

obsolete

>Index: WebCore/ChangeLog
>===================================================================
>--- WebCore/ChangeLog	(revision 53355)
>+++ WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,25 @@
>+2010-01-15  John Sullivan  <sullivan@apple.com>
>+
>+        https://bugs.webkit.org/show_bug.cgi?id=33751 and <rdar://problem/7538330>
>+        Zip code field is misidentified as street address because id attribute isn't checked.
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        No new tests. I believe this code is used only by Safari AutoFill, but in any case
>+        it does not affect page rendering or anything else at the WebCore/WebKit level.
>+
>+        * page/Frame.cpp:
>+        (WebCore::matchLabelsAgainstString):
>+        New function, split out from matchLabelsAgainstElement.
>+        (WebCore::Frame::matchLabelsAgainstElement):
>+        Now calls matchLabelsAgainstString for the id attribute if no match is found for the name attribute.
>+        
>+        * page/mac/FrameMac.mm:
>+        (WebCore::matchLabelsAgainstString):
>+        Same as above. This is a parallel copy of the function using Mac-specific data structures.
>+        (WebCore::Frame::matchLabelsAgainstElement):
>+        Ditto.
>+
> 2010-01-15  Carol Szabo  <carol.szabo@nokia.com>
> 
>         Reviewed by Darin Adler.
>Index: WebCore/page/Frame.cpp
>===================================================================
>--- WebCore/page/Frame.cpp	(revision 53276)
>+++ WebCore/page/Frame.cpp	(working copy)
>@@ -494,25 +494,26 @@ String Frame::searchForLabelsBeforeEleme
>     return String();
> }
> 
>-String Frame::matchLabelsAgainstElement(const Vector<String>& labels, Element* element)
>+static String matchLabelsAgainstString(const Vector<String>& labels, const String& stringToMatch)
> {
>-    String name = element->getAttribute(nameAttr);
>-    if (name.isEmpty())
>+    if (stringToMatch.isEmpty())
>         return String();
> 
>-    // Make numbers and _'s in field names behave like word boundaries, e.g., "address2"
>-    replace(name, RegularExpression("\\d", TextCaseSensitive), " ");
>-    name.replace('_', ' ');
>+    String mutableStringToMatch = stringToMatch;
> 
>+    // Make numbers and _'s in field names behave like word boundaries, e.g., "address2"
>+    replace(mutableStringToMatch, RegularExpression("\\d", TextCaseSensitive), " ");
>+    mutableStringToMatch.replace('_', ' ');
>+    
>     OwnPtr<RegularExpression> regExp(createRegExpForLabels(labels));
>-    // Use the largest match we can find in the whole name string
>+    // Use the largest match we can find in the whole string
>     int pos;
>     int length;
>     int bestPos = -1;
>     int bestLength = -1;
>     int start = 0;
>     do {
>-        pos = regExp->match(name, start);
>+        pos = regExp->match(mutableStringToMatch, start);
>         if (pos != -1) {
>             length = regExp->matchedLength();
>             if (length >= bestLength) {
>@@ -522,11 +523,24 @@ String Frame::matchLabelsAgainstElement(
>             start = pos + 1;
>         }
>     } while (pos != -1);
>-
>+    
>     if (bestPos != -1)
>-        return name.substring(bestPos, bestLength);
>+        return mutableStringToMatch.substring(bestPos, bestLength);
>     return String();
> }
>+    
>+String Frame::matchLabelsAgainstElement(const Vector<String>& labels, Element* element)
>+{
>+    // Match against the name element, then against the id element if no match is found for the name element.
>+    // See 7538330 for one popular site that benefits from the id element check.
>+    // FIXME: This code is mirrored in FrameMac.mm. It would be nice to make the Mac code call the platform-agnostic
>+    // code, which would require converting the NSArray of NSStrings to a Vector of Strings somewhere along the way.
>+    String resultFromNameAttribute = matchLabelsAgainstString(labels, element->getAttribute(nameAttr));
>+    if (!resultFromNameAttribute.isEmpty())
>+        return resultFromNameAttribute;
>+    
>+    return matchLabelsAgainstString(labels, element->getAttribute(idAttr));;
>+}
> 
> const VisibleSelection& Frame::mark() const
> {
>Index: WebCore/page/mac/FrameMac.mm
>===================================================================
>--- WebCore/page/mac/FrameMac.mm	(revision 53276)
>+++ WebCore/page/mac/FrameMac.mm	(working copy)
>@@ -229,25 +229,26 @@ NSString* Frame::searchForLabelsBeforeEl
>     return nil;
> }
> 
>-NSString* Frame::matchLabelsAgainstElement(NSArray* labels, Element* element)
>+static NSString *matchLabelsAgainstString(NSArray *labels, const String& stringToMatch)
> {
>-    String name = element->getAttribute(nameAttr);
>-    if (name.isEmpty())
>+    if (stringToMatch.isEmpty())
>         return nil;
>-
>+    
>+    String mutableStringToMatch = stringToMatch;
>+    
>     // Make numbers and _'s in field names behave like word boundaries, e.g., "address2"
>-    replace(name, RegularExpression("\\d", TextCaseSensitive), " ");
>-    name.replace('_', ' ');
>-
>+    replace(mutableStringToMatch, RegularExpression("\\d", TextCaseSensitive), " ");
>+    mutableStringToMatch.replace('_', ' ');
>+    
>     RegularExpression* regExp = regExpForLabels(labels);
>-    // Use the largest match we can find in the whole name string
>+    // Use the largest match we can find in the whole string
>     int pos;
>     int length;
>     int bestPos = -1;
>     int bestLength = -1;
>     int start = 0;
>     do {
>-        pos = regExp->match(name, start);
>+        pos = regExp->match(mutableStringToMatch, start);
>         if (pos != -1) {
>             length = regExp->matchedLength();
>             if (length >= bestLength) {
>@@ -257,12 +258,25 @@ NSString* Frame::matchLabelsAgainstEleme
>             start = pos + 1;
>         }
>     } while (pos != -1);
>-
>+    
>     if (bestPos != -1)
>-        return name.substring(bestPos, bestLength);
>+        return mutableStringToMatch.substring(bestPos, bestLength);
>     return nil;
> }
> 
>+NSString* Frame::matchLabelsAgainstElement(NSArray* labels, Element* element)
>+{
>+    // Match against the name element, then against the id element if no match is found for the name element.
>+    // See 7538330 for one popular site that benefits from the id element check.
>+    // FIXME: This code is mirrored in Frame.cpp. It would be nice to make the Mac code call the platform-agnostic
>+    // code, which would require converting the NSArray of NSStrings to a Vector of Strings somewhere along the way.
>+    String resultFromNameAttribute = matchLabelsAgainstString(labels, element->getAttribute(nameAttr));
>+    if (!resultFromNameAttribute.isEmpty())
>+        return resultFromNameAttribute;
>+    
>+    return matchLabelsAgainstString(labels, element->getAttribute(idAttr));
>+}
>+
> NSImage* Frame::imageFromRect(NSRect rect) const
> {
>     NSView* view = m_view->documentView();

Actions: View | Formatted Diff | Diff

Attachments on bug 33751: 46725 | 46726 | 46728