Last Comment Bug 659743 - Remove nsImageMapUtils
: Remove nsImageMapUtils
Status: VERIFIED FIXED
:
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla7
Assigned To: :Ms2ger
:
Mentors:
Depends on:
Blocks: 811428
  Show dependency treegraph
 
Reported: 2011-05-25 13:18 PDT by :Ms2ger
Modified: 2012-11-13 12:11 PST (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch v1 (26.29 KB, patch)
2011-05-25 13:18 PDT, :Ms2ger
bugs: review-
Details | Diff | Review
Part c: Cleanup in nsImageMap (5.44 KB, patch)
2011-05-25 14:17 PDT, :Ms2ger
bugs: review+
Details | Diff | Review
Part a: Support image maps according to spec in non-HTMLDocuments (14.98 KB, patch)
2011-05-27 13:15 PDT, :Ms2ger
bugs: review+
Details | Diff | Review
Part b: Remove nsImageMapUtils (12.97 KB, patch)
2011-05-27 13:17 PDT, :Ms2ger
bugs: review+
Details | Diff | Review

Description :Ms2ger 2011-05-25 13:18:47 PDT
Created attachment 535164 [details] [diff] [review]
Patch v1
Comment 1 :Ms2ger 2011-05-25 14:17:40 PDT
Created attachment 535181 [details] [diff] [review]
Part c: Cleanup in nsImageMap

And while here, I noticed some not-so-useful QI'ing
Comment 2 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-05-26 10:48:35 PDT
Comment on attachment 535181 [details] [diff] [review]
Part c: Cleanup in nsImageMap

>-class nsImageMap : public nsStubMutationObserver, public nsIDOMFocusListener
>+class nsImageMap : public nsStubMutationObserver
>+                 , public nsIDOMFocusListener
, should be in the previous line
Comment 3 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2011-05-27 02:32:30 PDT
Comment on attachment 535164 [details] [diff] [review]
Patch v1

>+Element*
>+nsDocument::FindImageMap(const nsAString& aUsemap)
>+{
>+  // We used to strip the whitespace from the usemap value as a Quirk,
>+  // but it was too quirky and didn't really work correctly - see bug
>+  // 87050
>+
>+  if (aUsemap.IsEmpty())
>+    return NULL;

if (aUsemap.IsEmpty()) {
  return nsnull;
}

>+
>+  nsAString::const_iterator start, end;
>+  aUsemap.BeginReading(start);
>+  aUsemap.EndReading(end);
>+
>+  PRInt32 hash = aUsemap.FindChar('#');
>+  if (hash < 0) {
>+    return NULL;
nsnull

>+  }
>+  // aUsemap contains a '#', set start to point right after the '#'
>+  start.advance(hash + 1);
>+
>+  if (start == end) {
>+    return NULL; // aUsemap == "#"
nsnull


>+  }
>+
>+  const nsAString& mapName = Substring(start, end);
>+
>+  if (!mImageMaps) {
>+    mImageMaps = new nsContentList(this, kNameSpaceID_XHTML, nsGkAtoms::map, nsGkAtoms::map);
>+  }
>+
>+  PRUint32 i, n = mImageMaps->Length(PR_TRUE);
>+  for (i = 0; i < n; ++i) {
>+    nsIContent* map = mImageMaps->GetNodeAt(i);
>+    if (map->AttrValueIs(kNameSpaceID_None, nsGkAtoms::id, mapName,
>+                         eCaseMatters) ||
>+        map->AttrValueIs(kNameSpaceID_None, nsGkAtoms::name, mapName,
>+                         eIgnoreCase)) {
>+      return map->AsElement();
>+    }
>+  }
>+
>+  return NULL;
>+}
In the new code all the documents are handled the same way


>-  nsCOMPtr<nsIHTMLDocument> htmlDoc(do_QueryInterface(aDocument));
>-  if (htmlDoc) {
>-    nsCOMPtr<nsIDOMHTMLMapElement> map =
>-      do_QueryInterface(htmlDoc->GetImageMap(usemap));
>-    return map.forget();
>-  } else {
>-    // For XHTML elements embedded in non-XHTML documents we get the
>-    // map by id since XHTML requires that where a "name" attribute
>-    // was used in HTML 4.01, the "id" attribute must be used in
>-    // XHTML. The attribute "name" is officially deprecated.  This
>-    // simplifies our life becase we can simply get the map with
>-    // getElementById().
>-    Element* element = aDocument->GetElementById(usemap);
>-
>-    if (element) {
>-      nsIDOMHTMLMapElement* map;
>-      CallQueryInterface(element, &map);
>-      return map;
>-    }
>-  }
But here we have different code for non-HTML documents

Why such change? The change to the behavior should be done in a different bug, and then the patch should contain
tests.
Comment 4 :Ms2ger 2011-05-27 13:15:04 PDT
Created attachment 535728 [details] [diff] [review]
Part a: Support image maps according to spec in non-HTMLDocuments
Comment 5 :Ms2ger 2011-05-27 13:17:09 PDT
Created attachment 535729 [details] [diff] [review]
Part b: Remove nsImageMapUtils
Comment 7 Eric Shepherd [:sheppy] 2011-08-08 11:19:21 PDT
This looks like it's really just a matter of noting that this went away for core app developers, and oughtn't to affect web developers at all. True?
Comment 8 Eric Shepherd [:sheppy] 2011-08-08 13:24:28 PDT
This is a pretty arcane case, and after talking to Ms2ger about it in IRC, we decided it's not worth documenting.

Note You need to log in before you can comment on or make changes to this bug.