Closed Bug 36997 Opened 24 years ago Closed 23 years ago

nested EMBED tag incorrectly placed if align=top on parent OBJECT


(Core :: Layout, defect, P2)






(Reporter: M.L.Hammond, Assigned: peterlubczynski-bugs)



(Keywords: html4, testcase, top100)


(5 files, 5 obsolete files)

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; m15)
BuildID:    2000041805

On the flash content plays fine, but on following the link
to "View with Flash 4.0" there's nothing to see, except for some motion in a
very thin band just under the location bar. Perhaps the movie is being displayed
off the screen?

Reproducible: Always
Steps to Reproduce:
1. View URL

Actual Results:  Content not properly displayed	

Expected Results:  Show the movie
I see the behaviour you mentioned. Seems like the ' align="left" ' tag is not 
being understood by the browser properly. Copying the source code locally and 
changing the value to 'align=left' works and the movie loads and plays fine.
oops..the original tag is "align=top". changing it to "align=left" works.
Even removing the "align=top" tag works, but I am wondering why we have to do order for the movie to play.
Changing the attribute to align=texttop makes it work. explains somewhat. Seems like it's
(technically) a problem with the page rather than the browser, but this doesn't
explain why it's mucking things up so badly.
 setting align=randomgarbage doesn't make it break, so it must be actively
getting it wrong :) (and "other browsers" can handle align=top) oddly, suggests that align=top
should be okay...
Thanks for the info. I believe this is not a plugin problem since the plugin 
plays properly. Reassigning to HTML element component. 
Assignee: av → rickg
Component: Plug-ins → HTML Element
QA Contact: shrir → petersen
I can reproduce the problem with the April 21 build. I noticed that the url 
contains a object with a nested EMBED. After some research, it looks like the 
object element is not able to render the flash animation so I passes to the EMBED 
element. If you comment out the EMBED element, the document will not rendered the 
flash src. The align="top" attribute is supported in object and is used to 
vertically align the top of object with the top current of the text line. I have 
a test case that works with align=top at
Ever confirmed: true
Andrei -- please take a look.
Assignee: rickg → av
Target Milestone: --- → M18
nsbeta2 6/1-. Marking flash. Shouldn't this be reassigned to whoever's 
fixing HTMLElement cmpn bugs? Who is that?
Keywords: flash, nsbeta2
Whiteboard: [nsbeta2-]
This bug has been marked "future" because the original netscape engineer working 
on this is over-burdened. If you feel this is an error, that you or another 
known resource will be working on this bug,or if it blocks your work in some way 
-- please attach your concern to the bug for reconsideration. 
Target Milestone: M18 → Future
massive update for QA contact.
QA Contact: petersen → lorca
Raising priority to P2--high profile backward compatibility for Flash content on 
the web. Clearing Future to -- and nominating nsbeta3.
Keywords: nsbeta3
Priority: P3 → P2
Target Milestone: Future → ---
Removing the align=top from the object element (which contains the embed element) 
will cause the flash anaimation to be rendered.
Marking nsbeta3+ and cc-ing attinasi, peterl. 
Adding rtm+.
Keywords: rtm
Whiteboard: [nsbeta2-] → [nsbeta2-] [rtm+]
PDT marking [rtm-] because there are workarounds
Whiteboard: [nsbeta2-] [rtm+] → [nsbeta2-] [rtm-]
Flash crasher or backward incompatibility bug. Nominating for nsbeta1 as
high-quality plug-in support is a priority for embedded applications.
Keywords: nsbeta1
REMOVING nsbeta2/3 and replacing with nsbeta1 KW.  Also clearing 
Status Whiteboard of nsbeta2/3 result.  We really should get Flash 
working.  Not that I like it, but other people do:).
Keywords: nsbeta2, nsbeta3
Whiteboard: [nsbeta2-] [rtm-] → [rtm-]
Attached file testcase
Assignee: av → buster
Summary: Flash movie doesn't play properly (wrong position?) → OBJECT tag incorrectly placed if align=top
Target Milestone: --- → mozilla0.8
This is caused by the align=top on the OBJECT tag. It's a floater but a 
placeholder isn't generated for it. 

Changing summary from: Flash movie doesn't play properly (wrong position?)

Sending it over to buster and nominating...
Reassigning QA Contact for all open and unverified bugs previously under Lorca's
care to Gerardo as per phone conversation this morning.
QA Contact: lorca → gerardok
Moving to Mozilla0.9
Target Milestone: mozilla0.8 → mozilla0.9
qa contact updated.
QA Contact: gerardok → bsharma
Assignee: buster → waterson
Target Milestone: mozilla0.9 → mozilla1.0
SPAM. HTML Element component deprecated, changing component to Layout. See bug
88132 for details.
Component: HTML Element → Layout
Keywords: html4, testcase
QA Contact: bsharma → moied
*** Bug 64645 has been marked as a duplicate of this bug. ***
Changing platform and OS to reflect the conditions of bug 64645, which has been
deemed a duplicate of this bug.
OS: Windows 98 → All
Hardware: PC → All
Blocks: 104166
*** Bug 105944 has been marked as a duplicate of this bug. ***
back to me....still happening....
Assignee: waterson → peterlubczynski
Whiteboard: [rtm-]
Target Milestone: mozilla1.0 → mozilla0.9.6
*** Bug 103438 has been marked as a duplicate of this bug. ***
Happens on PBS which I think is top100.

Hey Marc or Chris, do you have any tips on what could be causing this? Thanks!
Keywords: top100
Is the attribute mapping is working correctly (i.e.,
nsIContent::GetMappedAttributeImpact)? Setting |align={left|right}| are supposed
to float the tag (per GetImageAlignAttributeImpact). What is |align=top|
supposed to do?
Well, nsHTMLObjectElement.cpp calls nsGenericHTMLElement::MapAlignAttributeInto
which looks like it only deals with left and right. I don't see where
|align=top| would be even be mapped in for the IMG tag. According to W3C, this
should work for Objects like it does for Images? Does |align=top| work for images?
Well, the orrignal web site,, is gone! Can someone attach a
(non)-working testcase?

I don't think the problem is with the mapping anymore. With the last attached
testcase, it seems to work just fine.

I think the problem maybe when the OBJECT tag specified a align=top but NOT the
EMBED tag. Since we render the EMBED tag instead of the OBJECT in the nested case.

Should the EMBED check it's parent OBJECT for allignment?
QA Contact: moied → shrir
Oh yeah, that's a good testcase. It shows the problem with the OBJECT tag having
align=top and not the EMBED.
Summary: OBJECT tag incorrectly placed if align=top → nested EMBED tag incorrectly placed if align=top on parent OBJECT
This is very strange. How does align=top work? It looks like the "line" above
the BODY is getting a bogus y value of "-3255" which is causing everything else
to be wrong. However, the actual BODY looks like it has the right y value. I'm
attaching a frame dump of the tescase with a normal dump without align=top to
Here is a reduced testcase that does not require any plugins:

<object align="top">
 <img src="">
Removing the align=top on the OBJECT tag and looking in Viewer with frame
borders on, it looks like the line is the wrong height and cuts through the
middle of the image. In the debugger, I can see the height of the inline being
set to only the font metrics size:
There is comment there that says "the height of children do not affect our
height" and that code is pretty old so I'm not even sure I'm looking in the
right place.

cc:ing dbaron for some advice...
Target Milestone: mozilla0.9.6 → mozilla0.9.7
Well, that looks like the correct behavior for 'vertical-align: text-top' and
perhaps also 'vertical-align: top' (although I would argue it isn't) on an
inline element.  Maybe we shouldn't be mapping the ALIGN attribute on the OBJECT
element to 'vertical-align' when the element isn't replaced, though that seems
tricky.  Ian, any ideas?
Even though it's deprecated, 4.01 spec does mention align=top:

Should align=top be ignored on the object tag if the contents get rendered?
I may be dense, but I do not see why align=top is supposed to work the way it
currently is.  Section 13.7.4 quoted by Peter says for top: "top: means that the
top of the object should be vertically aligned with the top of the current text
line."  I cannot fathom any way in which "top of the object should be vertically
aligned with the top of the current text line" means to push the object way
beyond the top of the page as I see nothing that would say that the top of the
current text line is above the top of the page.  If you add some text next to
the image in Marc's example, the text appears fine so it seems that the top of
the current text line is well within the boundaries of the page.

...because the image sticks out of the top of the inline frame for the object.

However, one can argue about what 'vertical-align: top' should do in such a case
-- see my proposal at
this is clearly visible on the shockwave site(hence it's important to be fixed 
since a lot of users will notice it ) 

The football image at the bottom right is peeking outside the table border.
After looking at this again this weekend, I think I agree with dbaron. Maybe for
replaced object elements, we should drop the 'vertical-align:' style? IE 6 seems
to ignore it in this situation but we try to support it (try 'sub' or a length).
Could one of the layout guys maybe suggest a strategy? I was thinking of setting
a style rule in nsCSSFrameConstructor::CantRenderReplaceElement(), but I'm not
quite sure how to set it?
We have problems with other "align attributes like "right" on the object frame
when it's replaced as well. This patch removes any align attribute before
calling CantRenderReplaceElement() to match our behavior with IE.

This hack isn't really a "fix" for this bug, but it should make top100 sites
that show this problem look better. I'm wondering if I can get reviews for this
patch and then move this bug to post-1.0 for a better fix?
That seems like a bit too much of a hack.  Would it be possible to change the
MapAttributesIntoRule in content/html/content/src/nsHTMLObjectElement.cpp so
that it doesn't call nsGenericHTMLElement::MapAlignAttributeInto if the object
element is replaced?  Or does the information about whether the object is
replaced live only on the frame (that would seem bad)?
We don't know we are going to be replaced until the end of reflow, at which time
we post a CantRenderReplaceElement(). I can't think of a way the content knows
that the frame has been replaced. I guess I can try to put in a hook however, I
don't see any call to MapAttributesIntoRule for the TextReset after the point I
know I need to be replaced. Will that be a problem or should re-resolving style
and a reflow fix that?
I tried to change |MapAttributesIntoRule| to block the calling of
|nsGenericHTMLElement::MapAlignAttributeInto| but since this static mapping
function isn't a member of the content class, I didn't have anything to work
with. I need to at least get at the content so I can call |GetPrimaryFrameFor|.

So anyway, here a patch with a slightly different approach. I just reset the
vertical align style back to the default before we start to replace the object
frame. Please reivew.
There's an API for doing that that won't break things like inherit bits.  You
want to call nsIStyleContext::GetUniqueStyleData (and you'll need to extend
nsStyleContext::GetUniqueStyleData for the TextReset struct), and then modify
that struct.
Attached patch patch using GetUniqueStyleData (obsolete) — Splinter Review
This seems to work. Thanks for the tip David, could you r= this patch?
Attachment #60497 - Attachment is obsolete: true
Attachment #61057 - Attachment is obsolete: true
Comment on attachment 61210 [details] [diff] [review]
patch using GetUniqueStyleData

>+    nsStyleTextReset* text = 
>+      (nsStyleTextReset*)mStyleContext->GetUniqueStyleData(mPresContext,  eStyleStruct_TextReset);

r=dbaron if you change the above to a new-style cast:

nsStyleTextReset* text = NS_STATIC_CAST(nsStyleTextReset*,
  mStyleContext->GetUniqueStyleData(mPresContext, eStyleStruct_TextReset));
Attachment #61210 - Flags: review+
Attached patch correct patch (obsolete) — Splinter Review
Attachment #61210 - Attachment is obsolete: true
Attachment #61213 - Flags: review+
Keywords: review
Comment on attachment 61213 [details] [diff] [review]
correct patch

I think that 

Check that out. sr=attinasi if you do.
Attachment #61213 - Flags: superreview+
Attachment #61213 - Attachment is obsolete: true
Attachment #61550 - Flags: superreview+
Attachment #61550 - Flags: review+
Keywords: reviewmozilla0.9.7
Comment on attachment 61550 [details] [diff] [review]
really corrected patch

+    const nsStyleTextReset* reset = (const

This could be new-style (NS_STATIC_CAST), too. for 0.9.7 in any case.

Attachment #61550 - Flags: approval+
Blocks: 114455
Patch in trunk, marking FIXED.
Closed: 23 years ago
Keywords: fixed0.9.7
Resolution: --- → FIXED
The problem I reported with (NewsHour and Washington Week video) is now
fixed with Mozilla trunk 2001-12-14-03 on Win2000.
Embedding may want this because it effects top100 sites, nominating...
Keywords: edt0.9.4
removing URL that is now dead.
not quite fixed yet....this causes crashing in some other major plugin sites so
I just backed myself out until there can be a better fix... :(
Resolution: FIXED → ---
Attached patch check for null (obsolete) — Splinter Review
okay, this patch seems to be better, now checking for null....
Attachment #61550 - Attachment is obsolete: true
Attachment #61778 - Attachment is obsolete: true
Comment on attachment 61550 [details] [diff] [review]
really corrected patch

okay, I'm an idiot....this patch is fine. The crash I was seeing was because I
didn't re-link the content DLL so naturally it didn't pick-up my changes and
returned null. In fact, Stephen in comment #61 says this patch is good so I'll
try to land it again tonight. Sorry for the trouble....
Attachment #61550 - Attachment is obsolete: false
a=asa (on behalf of drivers) for checkin to 0.9.7
Nominating edt0.9.4
Keywords: edt0.9.4
Closed: 23 years ago23 years ago
Keywords: fixed0.9.7
Resolution: --- → FIXED
No longer blocks: 114455
Per shrir comment:
I suggest we include for 094. I would like to see it verified fixed on trunk/097.
Yes, this is fixed on trunk/0.9.7 branch build 1220. I tried all testcases/ 
duped bugs and the shockwave site as well. Looks good.
Plused for 094
Keywords: edt0.9.4edt0.9.4+
Keywords: fixed0.9.4
verif fixed on 0101 0.9.4 branch build.
Keywords: verified0.9.4
Keywords: fixed0.9.4
You need to log in before you can comment on or make changes to this bug.