Make marquee a block element to not break site layout

VERIFIED FIXED in mozilla1.0.1

Status

()

Core
HTML: Parser
P1
normal
VERIFIED FIXED
16 years ago
16 years ago

People

(Reporter: Doron Rosenberg (IBM), Assigned: harishd)

Tracking

({intl, topembed})

Trunk
mozilla1.0.1
x86
Windows 2000
intl, topembed
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

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

Attachments

(1 attachment, 1 obsolete attachment)

3.27 KB, patch
Heikki Toivonen (remove -bugzilla when emailing directly)
: review+
Heikki Toivonen (remove -bugzilla when emailing directly)
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

16 years ago
Site layout breaks because marquee is treated as a unkown elements and thus made
inline.
(Assignee)

Comment 1

16 years ago
Created attachment 89101 [details] [diff] [review]
patch v1.0

Allows marquee to behave as a block level element.
(Reporter)

Comment 2

16 years ago
*** Bug 153983 has been marked as a duplicate of this bug. ***
Didn't we already fix this in bug 147183?

Comment 4

16 years ago
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

Comment 5

16 years ago
add
valeski@netscape.com,chofmann@netscape.com,msanz@netscape.com,lyecies@netscape.com
to the cc list.
Keywords: topembed

Comment 6

16 years ago
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. 

Updated

16 years ago
Blocks: 143047
Keywords: nsbeta1 → nsbeta1+
Whiteboard: [adt1 RTM] [ETa Needed]
Target Milestone: --- → mozilla1.0.1

Comment 7

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

Updated

16 years ago
Blocks: 141008

Comment 9

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

Updated

16 years ago
Whiteboard: [adt1 RTM] [ETa Needed] → [adt1 RTM] [ETA 06/27]
(Assignee)

Updated

16 years ago
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+

Comment 11

16 years ago
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.
(Assignee)

Comment 12

16 years ago
>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.
(Assignee)

Comment 13

16 years ago
Created attachment 89286 [details] [diff] [review]
patch v1.1 [ addressing jst's concern ]
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+

Comment 15

16 years ago
pls land this on the trunk asap, and get it verified by QA, so that we can get
it on the branch tomorrow. thanks!
Keywords: approval, mozilla1.0.1
(Assignee)

Comment 16

16 years ago
Fix landed ( 06/26 ) on the trunk.
Whiteboard: [adt1 RTM] [ETA 06/27] → [adt1 RTM] [ETA 06/27][fixed on the trunk 06/26]

Comment 17

16 years ago
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]

Updated

16 years ago
Keywords: adt1.0.1

Comment 18

16 years ago
resolving as fixed since this has landed on the trunk. adt1.0.1 and mozilla1.0.1
have been added for branch nomination. 
(Reporter)

Comment 19

16 years ago
this is fixed on commercial trunk.
Status: ASSIGNED → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 20

16 years ago
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.1 → adt1.0.1+

Comment 21

16 years ago
please checkin to the 1.0.1 branch. once there, remove the "mozilla1.0.1+"
keyword and add the "fixed1.0.1" keyword.
Keywords: mozilla1.0.1 → mozilla1.0.1+
(Assignee)

Comment 22

16 years ago
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]

Updated

16 years ago
Keywords: mozilla1.0.1+

Updated

16 years ago
Blocks: 154896

Updated

16 years ago
No longer blocks: 154896

Updated

16 years ago
Blocks: 154896

Updated

16 years ago
Keywords: verified1.0.1
You need to log in before you can comment on or make changes to this bug.