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)
Core
CSS Parsing and Computation
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)
50 bytes,
text/html
|
Details | |
1.74 KB,
patch
|
Details | Diff | Splinter Review | |
4.24 KB,
patch
|
tbsaunde
:
review+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 1•14 years ago
|
||
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 }
Blocks: quirks-mode-spec
Assignee | ||
Comment 3•12 years ago
|
||
> 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.
Assignee | ||
Comment 4•12 years ago
|
||
Attachment #639759 -
Flags: review?(dbaron)
Assignee | ||
Updated•12 years ago
|
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+
Assignee | ||
Comment 7•12 years ago
|
||
Attachment #640082 -
Flags: review?(surkov.alexander)
Assignee | ||
Updated•12 years ago
|
Attachment #640082 -
Flags: review?(surkov.alexander) → review?(trev.saunders)
Updated•12 years ago
|
Keywords: dev-doc-needed
Comment 8•12 years ago
|
||
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)
Assignee | ||
Comment 9•12 years ago
|
||
> 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.
Comment 10•12 years ago
|
||
(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())
Assignee | ||
Comment 11•12 years ago
|
||
> 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?
Assignee | ||
Comment 12•12 years ago
|
||
Attachment #642477 -
Flags: review?(trev.saunders)
Assignee | ||
Updated•12 years ago
|
Attachment #639759 -
Attachment is obsolete: true
Comment 13•12 years ago
|
||
(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.
Assignee | ||
Comment 14•12 years ago
|
||
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!
Updated•12 years ago
|
Attachment #642477 -
Flags: review?(trev.saunders) → review+
Assignee | ||
Comment 15•12 years ago
|
||
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
Comment 16•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/cf567772a2f7
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 17•12 years ago
|
||
(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 → ---
Assignee | ||
Updated•12 years ago
|
Status: REOPENED → RESOLVED
Closed: 12 years ago → 12 years ago
Resolution: --- → FIXED
Comment 18•12 years ago
|
||
I dropped a not in: https://developer.mozilla.org/en-US/docs/Firefox_17_for_developers and https://developer.mozilla.org/en-US/docs/HTML/Element/map
Keywords: dev-doc-needed → dev-doc-complete
You need to log in
before you can comment on or make changes to this bug.
Description
•