531030 - Remove support for the <spacer> element

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

Gecko supports the <spacer> element, which is a Netscape extension to HTML. IE8, Safari 4 and Opera 10 don't support the element.

HTML5 previously had parser support for this element, but Hixie removed the parser support. I noticed this today, when updating the HTML5 parser to spec.

Having layout support for <spacer> is bad if the parser doesn't treat the element as a void element.

I think this leaves two reasonable options:
 1) Removing support for this legacy Netscapism that no one else implements from layout.
 2) Trying to get it (re-)formalized in HTML5 even though other browsers have gotten away without implementing it. (I'll pursue this if this bug gets WONTFIXed.)

(The third option of letting this be the only case where Gecko's HTML5 parser  deviates from the spec doesn't seem like a good option--at least not without pursuing the other two first.)

In July 2007, Philip Taylor analyzed dmoz-listed pages
http://canvex.lazyilluminati.com/survey/2007-07-17/analyse.cgi/index
and found the element on ~1% of pages analyzed. However, following links to those pages now shows that usage is on decline (some pages no longer have <spacer>). It seems that Adobe GoLive 4 has generated <spacer> elements.

Opera MAMA ranks <spacer> above <blink> in usage frequency (not clear if those are pages or instances of the tag):
http://dev.opera.com/articles/view/mama-phrase-block-list/

Comment 1

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

I should mention that I'm not particularly advocating a particular course of action here, but I'm more seeking the layout module owners' opinion. (One might argue that even if <spacer> weren't supported in layout, it would still make sense to parse it as a void element just in case, because parsing it that way is a low-cost thing to do.)

Comment 2

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

(In reply to comment #1)
> (One might
> argue that even if <spacer> weren't supported in layout, it would still make
> sense to parse it as a void element just in case, because parsing it that way
> is a low-cost thing to do.)

Filed a spec bug with this argument:
http://www.w3.org/Bugs/Public/show_bug.cgi?id=8372

Comment 3

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

(In reply to comment #0)
> In July 2007, Philip Taylor analyzed dmoz-listed pages
> http://canvex.lazyilluminati.com/survey/2007-07-17/analyse.cgi/index
> and found the element on ~1% of pages analyzed. However, following links to
> those pages now shows that usage is on decline (some pages no longer have
> <spacer>). It seems that Adobe GoLive 4 has generated <spacer> elements.

Even though the frequency of usage seems high, it doesn't seem to matter for perceived Web compat:
http://www.w3.org/Bugs/Public/show_bug.cgi?id=8372#c4

Comment 4

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

Opinions, considering http://www.w3.org/Bugs/Public/show_bug.cgi?id=8372#c4 ?

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Let's get rid of it.

Comment 6

[Boris Zbarsky [:bzbarsky]]

In the words of Mortal Kombat: Finish it.

Comment 7

[(no longer active)]

Is there really anything layout specific about spacers?  As far as I can tell, they're just static positioned boxes, right?  I think we should move this bug to Core::DOM.

Comment 8

[Boris Zbarsky [:bzbarsky]]

> Is there really anything layout specific about spacers? 

Yes.  There's nsSpacerFrame.cpp (and the bits in nsCSSFrameConstructor to create it).  The DOM part is very very small compared to that.

Comment 9

[(no longer active)]

Oh, I stand corrected then.

Comment 10

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: WIP

Still need to test.

Comment 11

[(no longer active)]

You should probably add a bunch of reftests using an idea like this:

test-1.html:
<p>foo</p>
<spacer height="100">
<p>bar</p>

test-ref.html:
<p>foo</p>
<p>bar</p>

Comment 12

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch v1

Didn't find any tests for spacers.

Comment 13

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Still need to remove SpacerFrame_id from layout/generic/nsQueryFrame.h

Comment 14

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 462088 [details] [diff] [review]
Patch v1

r=bzbarsky

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 462088 [details] [diff] [review]
Patch v1

a2.0=dbaron

Comment 16

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch for checkin

Comment 17

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/45716b17fb82

Comment 18

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

<https://developer.mozilla.org/en/Firefox_4_for_developers#section_3>