Closed
Bug 659743
Opened 12 years ago
Closed 12 years ago
Remove nsImageMapUtils
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
VERIFIED
FIXED
mozilla7
People
(Reporter: Ms2ger, Assigned: Ms2ger)
References
Details
Attachments
(3 files, 1 obsolete file)
5.44 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
14.98 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
12.97 KB,
patch
|
smaug
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Attachment #535164 -
Flags: review?(Olli.Pettay)
Assignee | ||
Comment 1•12 years ago
|
||
And while here, I noticed some not-so-useful QI'ing
Attachment #535181 -
Flags: review?(Olli.Pettay)
Comment 2•12 years ago
|
||
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
Attachment #535181 -
Flags: review?(Olli.Pettay) → review+
Comment 3•12 years ago
|
||
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.
Attachment #535164 -
Flags: review?(Olli.Pettay) → review-
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #535164 -
Attachment is obsolete: true
Attachment #535728 -
Flags: review?(Olli.Pettay)
Assignee | ||
Comment 5•12 years ago
|
||
Attachment #535729 -
Flags: review?(Olli.Pettay)
Updated•12 years ago
|
Attachment #535728 -
Flags: review?(Olli.Pettay) → review+
Updated•12 years ago
|
Attachment #535729 -
Flags: review?(Olli.Pettay) → review+
Assignee | ||
Comment 6•12 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/7a7aea19dd85 http://hg.mozilla.org/mozilla-central/rev/54eecb1a8614 http://hg.mozilla.org/mozilla-central/rev/bef2d3989289
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla7
Assignee | ||
Updated•12 years ago
|
Keywords: dev-doc-needed
Assignee | ||
Updated•12 years ago
|
Attachment #535181 -
Attachment description: Cleanup in nsImageMap → Part c: Cleanup in nsImageMap
Comment 7•12 years ago
|
||
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•12 years ago
|
||
This is a pretty arcane case, and after talking to Ms2ger about it in IRC, we decided it's not worth documenting.
Keywords: dev-doc-needed
Comment 9•12 years ago
|
||
content/html/content/test/test_bug659743.xml passed on all the OSs: https://tbpl.mozilla.org/php/getParsedLog.php?id=6420477&full=1 https://tbpl.mozilla.org/php/getParsedLog.php?id=6420362&full=1 https://tbpl.mozilla.org/php/getParsedLog.php?id=6420285&full=1 https://tbpl.mozilla.org/php/getParsedLog.php?id=6420482&full=1
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•