Build: 2002-03-26-08 Platform: All Expected Results: Param element's value attribute should be displayed by applet element. Applet should appear with a blue background with scrolling phrase displaying "scrolling banner text". What I got: Param value attribute isn't passing this information to the applet. Applet appears with a white background and scrolling phrase stating "Message not Found". Steps to reproduce: 1) First, go to HTML version of the document: http://mozilla.org/quality/browser/standards/html/applet_align_top.html 2) Applet should appear with a blue background with scrolling phrase displaying "scrolling banner text". 3) Go to the xhtml version: http://mozilla.org/quality/browser/standards/xhtml/transitional/applet_align_top.xml 4) Applet appears with a white background and scrolling phrase stating "Message not Found".

The applet in the xhtml document is rendered correctly in NS 6.2.1.

minusing for now. please re-nominate if we've miss-perceived the frequency and end user impact.

Removed minus from nsbeta1. Needs to be resolved. This is currently working in NS 6.2.3.

Um, this is not working with todays trunk build, nor can I see how it ever worked in any recent builds (i.e. I really doubt it works on the mozilla1.0 branch either, but I don't have a build handy right now where I could test). Untested fix coming up.

Comment on attachment 84464 [details] [diff] [review] Proposed fix, don't look for "PARAM" elements in XHTML, take the namespace into account. Tons for cleanup too... >Index: layout/html/base/src/nsObjectFrame.cpp >=================================================================== >- mydomElement->GetElementsByTagName(NS_LITERAL_STRING("PARAM"), getter_AddRefs(allParams)); Doh! I think I need to search LXR for all use of all DOM methods and see if they should be fixed to do what you did here... >- if (parent.get() == mydomNode.get()) { >+ if (parent == mydomElement) { This is the only part that worries me. In the case of interface pointers, shouldn't we compare the same interface pointers (the old code was comparing nsIDOMNodes, your patch compares nsIDOMNode to nsIDOMElement)?

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

Adding custrtm- to comment fields. No impact on customization.

Oh, hmm, I wasn't cc:d here so I didn't see that question earlier... In theory, yeah, we should be comparing interface pointers of the same type, that's the right thing to do, and I'm fine with you putting that back when checking in. But my patch will do the right thing in mozilla since no DOM element implementations use a different interface pointer for nsIDOMElement and nsIDOMNode. And since speed doesn't really matter here, we might as well do the right thing and be safe in the extremely unlikely event that someone would change the interface inheritance model for HTML param elements...

Ok, these are the only changes to jst's patch. + nsCOMPtr<nsIDOMNode> mydomNode = do_QueryInterface(mydomElement); + NS_ENSURE_TRUE(mydomNode, NS_ERROR_NO_INTERFACE); ! if (parent.get() == mydomNode.get()) {

Comment on attachment 88518 [details] [diff] [review] Fixes my issue r=heikki

removing adt1.0.1 until this has sr=, and has been verified on the trunk.

Comment on attachment 88518 [details] [diff] [review] Fixes my issue r=peterv. Changing heikki's r to an sr.

Comment on attachment 88518 [details] [diff] [review] Fixes my issue >+ nsCOMPtr<nsIDOMNode> mydomNode = do_QueryInterface(mydomElement); >+ NS_ENSURE_TRUE(mydomNode, NS_ERROR_NO_INTERFACE); >+ if (parent.get() == mydomNode.get()) { Please loose those .get()'s when checking this in, and the NS_ENSURE_TRUE should be replaced with an NS_ASSERTION (if anything at all), that really should never happen, but even if it does the code would do the right thing even w/o the NS_ENSURE_TRUE(), we'll save a few instructions... sr=jst for this part of the change.

Fixed on trunk with last of jst's nits, will attach as well.

Comment on attachment 88672 [details] [diff] [review] final Carrying reviews forward.

Resolving as fixed per Comment #15 From Heikki Toivonen. chris - can you pls verify this on the trunk? thanks!

Excellent.... Verified on the June 24th trunk builds: OS X (2002-06-24-08) Linux (2002-06-24-08) Windows ME (2002-06-24-08)

Any reason why this isn't checked into the branch yet ? I would like to see this get resolved soon.

This has not been checked in to the branch because it does not have the needed approvals. Cc members of ADT & drivers in the hopes that we'd get them...

It would nice to get approval since this is considered a regression. Works properly in NS 6.2.

> Cc members of ADT & drivers in the hopes that we'd get them... Heh. 'jrgm' is neither ADT nor driver. Maybe you were looking for 'jaimejr' :-)

resolving as verified per Comment #19 From Chris Petersen.

This had not been approved because it had not been resolved/verified. Marked it as so, and will pursue getting the approvals. thanks!

Shh! You exposed my secret plot. You could have pretented (I really need spell checker for text areas, my mind is gone...) to be Jaime and given approval ;)

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

adt1.0.1- per the ADT. let's get it in the next release.