154173 - Make marquee a block element to not break site layout

Status: VERIFIED FIXED

(Core :: DOM: HTML Parser, defect, P1)

Description

[Doron Rosenberg (IBM)]

Site layout breaks because marquee is treated as a unkown elements and thus made
inline.

Comment 1

[harishd]

Attachment: patch v1.0

Allows marquee to behave as a block level element.

Comment 2

[Doron Rosenberg (IBM)]

*** Bug 153983 has been marked as a duplicate of this bug. ***

Comment 3

[Simon Montagu :smontagu]

Didn't we already fix this in bug 147183?

Comment 4

[Frank Tang]

simon, the other fix does not address the parser problem. 
jaimejr, this is the bug we really need to get into MachV. According to harishd,
the patch should have no risk for page which do not have <marquee> tag init. 

Comment 5

[Frank Tang]

add
[e-mail],[e-mail],[e-mail],[e-mail]
to the cc list.

Comment 6

[Frank Tang]

This fix is also needed if we have the following
<marquee>
  <div>
  </div>
</marquee>
if we want to find the first childrean of the marquee through DOM, without this
patch, it won't find the div. That is violation of the DOM spec. 
The fix move the treatment of <marquee> in the html parser code from one
undefined behavior to aonther undefine behavior in term of html (change from
inline to block). But it fix the DOM problem according to the DOM specification. 
Also, it is very very very very important for one of our top embedding issue. 

Comment 7

[Frank Tang]

doron, what is the bugscape bug number we should look at? please put down the
bug number here. thanks.

Comment 8

[Doron Rosenberg (IBM)]

http://bugscape.mcom.com/show_bug.cgi?id=17067

Comment 9

[Frank Tang]

The patch is here. We need to got r= and sr= quickly and land into the trunk.

Comment 10

[Johnny Stenback (:jst)]

Comment on attachment 89101 [details] [diff] [review]
patch v1.0

What kind of element class do we end up creating for this new tag?
nsHTMLUnknownElement? If so, we won't get any inline style on these marquee
elements, and that would probably be a bad thing, no?

IOW, don't we need to hook the sink up to create some more sane type of element
class for marquee elements? Maybe just a div (i.e. a nsHTMLDivElement with the
name "marquee") would be good enough, or do we need more than that?

sr=jst for the parser changes at least.

Comment 11

[Frank Tang]

jst: thanks for your sr=. Let's focus one thing at a time. I think marquee
defined by some other company as block instead of inline. But that is an
seperate issue. 
heikki- can you r= this one? this is one of the top priority now. thanks.

Comment 12

[harishd]

>What kind of element class do we end up creating for this new tag?
>nsHTMLUnknownElement? If so, we won't get any inline style on these marquee
>elements, and that would probably be a bad thing, no?

We end up creating NS_NewHTMLSpanElement element!. I'm not sure if that's a bad
thing or not. But, you're right may be we need to create nsHTMLDivElement if
marquee really wants to behave as a block.

Comment 13

[harishd]

Attachment: patch v1.1 [ addressing jst's concern ]

Comment 14

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Comment on attachment 89286 [details] [diff] [review]
patch v1.1 [ addressing jst's concern ]

r=heikki, and carrying forward sr from jst.

Comment 15

[Jaime Rodriguez, Jr.]

pls land this on the trunk asap, and get it verified by QA, so that we can get
it on the branch tomorrow. thanks!

Comment 16

[harishd]

Fix landed ( 06/26 ) on the trunk.

Comment 17

[Jaime Rodriguez, Jr.]

doron - pls verify this on the trunk tomorrow. thanks!

Comment 18

[Jaime Rodriguez, Jr.]

resolving as fixed since this has landed on the trunk. adt1.0.1 and mozilla1.0.1
have been added for branch nomination. 

Comment 19

[Doron Rosenberg (IBM)]

this is fixed on commercial trunk.

Comment 20

[Jaime Rodriguez, Jr.]

adt1.0.1+ (on ADT's behalf) approval for checkin to the 1.0 branch, pending
drivers' approval. pls check this in asap. thanks!

Comment 21

[Judson Valeski]

please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.

Comment 22

[harishd]

Fix landed ( 06/28 ) on the branch.