Closed Bug 154173 Opened 17 years ago Closed 17 years ago

Make marquee a block element to not break site layout

Categories

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

x86
Windows 2000
defect

Tracking

()

VERIFIED FIXED
mozilla1.0.1

People

(Reporter: doronr, Assigned: harishd)

References

()

Details

(Keywords: intl, topembed, Whiteboard: [adt1 RTM] [ETA 06/27][fixed on the trunk 06/26 and brach 06/28])

Attachments

(1 file, 1 obsolete file)

Site layout breaks because marquee is treated as a unkown elements and thus made
inline.
Attached patch patch v1.0 (obsolete) — Splinter Review
Allows marquee to behave as a block level element.
*** Bug 153983 has been marked as a duplicate of this bug. ***
Didn't we already fix this in bug 147183?
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. 
Keywords: intl, nsbeta1
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. 
Blocks: 143047
Keywords: nsbeta1nsbeta1+
Whiteboard: [adt1 RTM] [ETa Needed]
Target Milestone: --- → mozilla1.0.1
doron, what is the bugscape bug number we should look at? please put down the
bug number here. thanks.
Blocks: 141008
The patch is here. We need to got r= and sr= quickly and land into the trunk.
Whiteboard: [adt1 RTM] [ETa Needed] → [adt1 RTM] [ETA 06/27]
Status: NEW → ASSIGNED
Priority: -- → P1
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.
Attachment #89101 - Flags: superreview+
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.
>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.
Attachment #89101 - Attachment is obsolete: true
Comment on attachment 89286 [details] [diff] [review]
patch v1.1 [ addressing jst's concern ]

r=heikki, and carrying forward sr from jst.
Attachment #89286 - Flags: superreview+
Attachment #89286 - Flags: review+
pls land this on the trunk asap, and get it verified by QA, so that we can get
it on the branch tomorrow. thanks!
Fix landed ( 06/26 ) on the trunk.
Whiteboard: [adt1 RTM] [ETA 06/27] → [adt1 RTM] [ETA 06/27][fixed on the trunk 06/26]
doron - pls verify this on the trunk tomorrow. thanks!
Whiteboard: [adt1 RTM] [ETA 06/27][fixed on the trunk 06/26] → [adt1 RTM] [ETA 06/27][fixed on the trunk 06/28]
Keywords: adt1.0.1
resolving as fixed since this has landed on the trunk. adt1.0.1 and mozilla1.0.1
have been added for branch nomination. 
this is fixed on commercial trunk.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
adt1.0.1+ (on ADT's behalf) approval for checkin to the 1.0 branch, pending
drivers' approval. pls check this in asap. thanks!
Status: RESOLVED → VERIFIED
Keywords: adt1.0.1adt1.0.1+
please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.
Fix landed ( 06/28 ) on the branch.
Keywords: fixed1.0.1
Whiteboard: [adt1 RTM] [ETA 06/27][fixed on the trunk 06/28] → [adt1 RTM] [ETA 06/27][fixed on the trunk 06/26 and brach 06/28]
Keywords: mozilla1.0.1+
Blocks: 154896
No longer blocks: 154896
Blocks: 154896
Keywords: verified1.0.1
You need to log in before you can comment on or make changes to this bug.