Closed Bug 607267 Opened 14 years ago Closed 12 years ago

Image maps are display:block, but the HTML spec says they should be inline

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla17

People

(Reporter: bugzilla, Assigned: bzbarsky)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete, html5)

Attachments

(3 files, 1 obsolete file)

I came across a website today, which used image maps like so: <img usemap=#m1 ...><map name=m1>...</map><img usemap=#m2 ...><map name=m2>...</map>. The site expected the two images to be displayed on the same line, but in Firefox they were displayed on top of each other. In Internet Explorer 9 in all four document modes, the two images are displayed on one line.

In http://mxr.mozilla.org/mozilla-central/source/layout/style/html.css#129 Firefox declares the <map> element as a block level element. Bug 57054 made <map> inline for quirks mode documents, but from what I can read in the standards, <map> should be inline in standards mode as well.

From HTML 4:
http://www.w3.org/TR/html4/sgml/dtd.html#inline
http://www.w3.org/TR/html4/sgml/dtd.html#special

From HTML 5:
http://www.w3.org/TR/html5/rendering.html#display-types
Keywords: html5
David, any objections to just making this change?
No other browser has it (WebKit, Opera, IE8).
Was there ever a standard who required this? It's not in CSS 2.1 Appendix D.

And it would save one rule in quirks.css:

  map { display: inline }
> Was there ever a standard who required this?

HTML4 allows <map> to contain %block; which is a bit silly if it's not a block itself.

That said, I think we should just make this change.
Attached patch Don't style imagemaps as block. (obsolete) — Splinter Review
Attachment #639759 - Flags: review?(dbaron)
Assignee: nobody → bzbarsky
Whiteboard: [need review]
Comment on attachment 639759 [details] [diff] [review]
Don't style imagemaps as block.

r=dbaron
Attachment #639759 - Flags: review?(dbaron) → review+
Attachment #640082 - Flags: review?(surkov.alexander)
Attachment #640082 - Flags: review?(surkov.alexander) → review?(trev.saunders)
Comment on attachment 640082 [details] [diff] [review]
a11y test fixes to deal with this change


>+      // map group.  Imagemaps are inlines by default, so TEXT_CONTAINER.
>       accTree =
>-        { PARAGRAPH: [
>+        { TEXT_CONTAINER: [

this part looks correct.

>+++ b/accessible/tests/mochitest/treeupdate/test_imagemap.html
>@@ -327,17 +327,19 @@
>       }
> 
>       this.finalCheck = function insertMap_finalCheck()
>       {
>         var accTree =
>           { SECTION: [
>             { IMAGE_MAP: [
>               { LINK: [ ] }
>-            ] }
>+            ] },
>+            // And a TEXT_LEAF for the imagemap itself
>+            { TEXT_LEAF: [] }

this seems wrong.  The comment sort of explains it, the accessible for the imagemap should be the object with role IMAGE_MAP and it shouldn't have a second accessible that's a text leaf.  I'm not sure exactly which case of creating a accessible by frame type is triggering this in nsAccessibilityService.cpp.  The solution is probably to add some sort of special case there or in nsTextFrame::CreateAccessible() does it make sense that the frame for an image map is now a nsTextFrame?

sorry about the delay in looking at this.
Attachment #640082 - Flags: review?(trev.saunders)
> the accessible for the imagemap should be the object with role IMAGE_MAP

That's the accessible for the _image_.

> I'm not sure exactly which case of creating a accessible by frame type is triggering this
> in nsAccessibilityService.cpp.

I could step through it, I guess.  I'll take a look and comment with the results.

> does it make sense that the frame for an image map is now a nsTextFrame?

It's not.  It's an nsInlineFrame, but probably contains an nsTextFrame.
(In reply to Boris Zbarsky (:bz) from comment #9)
> > the accessible for the imagemap should be the object with role IMAGE_MAP
> 
> That's the accessible for the _image_.

true, but perhaps I should be more precise.  I think the idea here was that in the accessible tree the image map and the image should be one object.  So the tree would look like

{ <div containing image map> {
  { accessible for image (is image map instead of normal image accessible object since there'll be kids) [
    <accessibles for areas>
  ]}
]}

sorry about the js diagram, I need to learn to ascii art this sort of thing.

The idea being people who look at the accessible tree don't need to think about the image and map being different things.

> > I'm not sure exactly which case of creating a accessible by frame type is triggering this
> > in nsAccessibilityService.cpp.
> 
> I could step through it, I guess.  I'll take a look and comment with the
> results.

ok, thanks!

> > does it make sense that the frame for an image map is now a nsTextFrame?
> 
> It's not.  It's an nsInlineFrame, but probably contains an nsTextFrame.

ok, I'd say certanly since nsTextFrames are the only thing that get TextLeafAccessibles (the only overload of nsIFrame::CreateAccessible() to call nsAccessibilityService::CreateHTMLTextLeafAccessible())
> I could step through it, I guess.  I'll take a look and comment with the results.

OK, so the story is that the test has this markup:

  <div id="container">
    <img id="imgmap" width="447" height="15"
         usemap="#atoz_map"
         src="../letters.gif">
  </div>

and then it creates a <map> element and does:

  this.containerNode.appendChild(map);

where this.containerNode is the <div id="container"> and "map" is the <map>.  The resulting frametree, with this patch, looks like this:

  Block(div)(10)@0x12c7f2670 [content=0x11fee7500] {0,3732,55440,1236} [state=0000120000100010] sc=0x12c7708a0(i=1,b=0)<
    line 0x12c7f39b0: count=3 state=inline,clean,prevmarginclean,not impacted,not wrapped,before:nobr,after:nobr[0x100] {0,0,26820,1236} <
      ImageFrame(img)(1)@0x12c7f3908 {0,0,26820,900} [state=000000c000300010] [content=0x12f5f2900] [sc=0x12c7f3868] [src=chrome://mochitests/content/a11y/accessible/letters.gif]
      Text(2)"\n  "@0x12d208030 [run=0x127b33080][0,3,T]  next=0x12c7f2518 {26820,180,0,960} [state=00000000c8400010] [content=0x11fee7680] sc=0x127cf6e08 pst=:-moz-non-element
      Inline(map)(3)@0x12c7f2518 {26820,180,0,960} [state=0000100000000000] [content=0x12e40bb30] [sc=0x12c76f9e8]<>
    >
  >

and the TEXT_LEAF is the accessible for that nsTextFrame containing the text "\n  ".  When the <map> used to be a block, frame construction optimized that textframe away, because it was on a block boundary, but now it gets created....

Trevor, thoughts?
Attachment #642477 - Flags: review?(trev.saunders)
Attachment #639759 - Attachment is obsolete: true
(In reply to Boris Zbarsky (:bz) from comment #11)

> and the TEXT_LEAF is the accessible for that nsTextFrame containing the text
> "\n  ".  When the <map> used to be a block, frame construction optimized
> that textframe away, because it was on a block boundary, but now it gets
> created....

if it's visible then there's no problem, if it's not then it should be fixed. Comment in the patch says "random textframes", it confuses.
Whether it's visible or not is a non-local decision: it depends on the styles of the <map>, on what comes after the <map>, and so forth.

Suggestions for what the comment should say are welcome!
Attachment #642477 - Flags: review?(trev.saunders) → review+
I took out the "random" bit and clarified why we want to avoid the textframes.

https://hg.mozilla.org/integration/mozilla-inbound/rev/cf567772a2f7
Flags: in-testsuite+
Whiteboard: [need review]
Target Milestone: --- → mozilla17
https://hg.mozilla.org/mozilla-central/rev/cf567772a2f7
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
(In reply to Boris Zbarsky (:bz) from comment #14)
> Whether it's visible or not is a non-local decision: it depends on the
> styles of the <map>, on what comes after the <map>, and so forth.

I see

(In reply to Boris Zbarsky (:bz) from comment #15)
> I took out the "random" bit and clarified why we want to avoid the
> textframes.
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/cf567772a2f7

thank you
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Closed: 12 years ago12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: