Closed Bug 264624 Opened 16 years ago Closed 15 years ago

Some imagemaps are broken here but not in IE

Categories

(Core :: DOM: Core & HTML, defect)

x86
All
defect
Not set
minor

Tracking

()

RESOLVED FIXED

People

(Reporter: S.P.Roach, Assigned: mats)

References

()

Details

(Keywords: compat, testcase, Whiteboard: QUIRK?)

Attachments

(4 files, 3 obsolete files)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20040910
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20040910

Go to this page.  http://www.eatyourlipstick.com/LelainaChapterTwo.htm (A
webcomic.) and you'll notice that the imagemaps don't all work.  A perusal of
them showed that the ones that DO work, have a single declaration of the map
name.  The one that had a single working button, had a second declaration of the
map name, AFTER that one working,(actually two buttons, with the same
destination,) button.  The broken buttons, in the imagemaps, all follow a second
copy of the map name declaration.

I can only HOPE I sent this bug to the right spot.

My computing environment is a 1.4GHz AMD, running Windows XP Pro Version 2002,
with service pack 2.

I have also sent the website's Sysop an email informing her of the "broken"
links, and probable fix.

Reproducible: Always
Steps to Reproduce:
1. Visit a webpage with imagemaps, that have duplicate declarations of the map name
2. notice the lower bar never changes from coordinate numbers to a link address
3.  notice that the labeled spot on the picture, that SHOULD lead you to a
certain page, does nothing.

Actual Results:  
When the cursor is over ANY button in the top bar, EXCEPT the one leading to the
main page for that particular webcomic, the lower bar will show coordinates, and
clicking does nothing.  The one that it does work on, the declaration for that
button comes before a duplicate entry of the map name declaration

Expected Results:  
All four, (actually at least eight,) regions should have worked, leading to 4
different pages.  One for the main homepage, one for the webcomic's main page,
one for news, and one for portfolio.

Go to this page.  http://www.eatyourlipstick.com/LelainaChapterTwo.htm (A
webcomic.) and you'll notice that the imagemaps don't all work.  A perusal of
them showed that the ones that DO work, have a single declaration of the map
name.  The one that had a single working button, had a second declaration of the
map name, AFTER that one working,(actually two buttons, with the same
destination,) button.  The broken buttons, in the imagemaps, all follow a second
copy of the map name declaration.

I can only HOPE I sent this bug to the right spot.

My computing environment is a 1.4GHz AMD, running Windows XP Pro Version 2002,
with service pack 2.

I have also sent the website's Sysop an email informing her of the "broken"
links, and probable fix.

Initially, I posted this to mozdev by mistake.  #7654.  Since invalidated
I blame some formatting issues on laziness, others on lack of sleep.  Initially
typed up for mozdev's bug tracker, which has slightly different fields.  I
didn't intend to paste the main body in twice, however.
Summary: Some imagemaps are broken here but not in IE → Some imagemaps are broken here but not in IE
Attached file Testcase (obsolete) —
Attached file Testcase
Attachment #162294 - Attachment is obsolete: true
The parser auto-closes the first MAP when it sees the second MAP start tag
and when there are more than one element with the same NAME attribute we
use the first. I think this is the correct behaviour per the HTML spec.
However, the link works in Opera, Konqueror and Netscape 4.x so I'm probably
missing something...
Severity: normal → minor
OS: Windows XP → All
Attached file Testcase (Quirks mode)
Attached patch Patch rev. 1 (obsolete) — Splinter Review
Something like this perhaps...
Whiteboard: QUIRK?
So if you add an <area> to that first map then it breaks in the other browsers too?
Yes: IE6/win32 and Opera7.54/linux.
No: Konqueror3.1.4/linux.
The reason why Konqueror works seems to be that it prefers the last MAP with
a mathing NAME. I also tested MAPs using the same NAME with valid AREAs but
using different URLs - all browser used the first URL except Konqueror that
used the last URL.
So, the patch would make us compatible with IE6 and Opera7 in Quirks mode.
Comment on attachment 162304 [details] [diff] [review]
Patch rev. 1

>Index: content/html/document/src/nsHTMLDocument.cpp

>+        rv = map->GetAreas(getter_AddRefs(mapAreas));
>+        if (NS_SUCCEEDED(rv) && mapAreas) {
>+          PRUint32 length = 0;
>+          mapAreas->GetLength(&length);
>+          if (length == 0) {

Couldn't you replace this with:

  PRBool hasKids = PR_FALSE;
  map->HasChildNodes(&hasKids);
  if (!hasKids) {

That should be a good deal faster and equivalent, since I doubt the tag-soup
parser allows anything but an <area> as a child of <map> and for XML documents
we'll never hit this code.

Other than that, this looks reasonable.
(In reply to comment #9)
> That should be a good deal faster and equivalent, since I doubt the tag-soup
> parser allows anything but an <area> as a child of <map> and for XML documents

The HTML4 spec allows a lot more than area as a child of map, so I sure hope our
parser does too.  And I think I've checked that it did in the past.  See, e.g.,
http://www.w3.org/TR/1999/REC-html401-19991224/struct/objects.html#h-13.6.1.1
Ah, true (just tested it, and <p> is allowed in an imagemap).  How do IE and
Opera treat imagemaps with only non-<area> children here?
And I guess there is the issue of child textnodes.... so the patch as posted may
be the best way to go.  :(
Attached patch Patch rev. 2 (obsolete) — Splinter Review
I have tested a lot more cases and this is the closest I can get to what
IE/Opera does without changing Parser/Frame stuff...  The way IE/Opera does
things is just weird - they allow MAP as a container and then all contained
AREAs seems to be associated with *all* currently open MAPs.

This patch is the same as rev. 1 except that I now keep looping while MAPs
are empty (as I said in the comment of the first patch but didn't do in the
code). For normal pages it should not be a performance impact by that
compared with the first patch.

I'm not sure if this is really worth fixing when there is a whole range of
other (tag soup) cases where we aren't compatible. Or maybe we should just
do what IE/Opera does in quirks mode?
Attachment #162304 - Attachment is obsolete: true
Attached file Testcase #3 - tag soup
Both left and right link areas work in IE6 and Opera7.
I'd rather not break parsing for this.  That patch seems reasonable as a quirk, yes.
Product: Browser → Seamonkey
Taking so I don't forget it again...
Assignee: general → mats.palmgren
Status: UNCONFIRMED → NEW
Component: General → DOM
Ever confirmed: true
Product: Mozilla Application Suite → Core
QA Contact: general → ian
Attached patch Patch rev. 3Splinter Review
As rev. 2 - just updated to trunk + removed an unused variable (unrelated)
to fix a compiler warning.
Attachment #162329 - Attachment is obsolete: true
Attachment #190310 - Flags: superreview?(bzbarsky)
Attachment #190310 - Flags: review?(bzbarsky)
Attachment #190310 - Flags: superreview?(bzbarsky)
Attachment #190310 - Flags: superreview+
Attachment #190310 - Flags: review?(bzbarsky)
Attachment #190310 - Flags: review+
Attachment #190310 - Flags: approval1.8b4?
Attachment #190310 - Flags: approval1.8b4? → approval1.8b4+
Checked in to trunk 2005-07-25 16:37 PDT

-> FIXED
Status: NEW → RESOLVED
Closed: 15 years ago
Keywords: qawanted
Resolution: --- → FIXED
Flags: in-testsuite?
Depends on: 587469
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.