93649 - document.plugins === document.embeds in NN4 while document.plugins === navigator.plugins in Mozilla

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Martin Honnen]

I stumbled today over the following: in NN4.06+
  document.plugins === document.embeds
evals to true while in Mozilla  we have
  document.plugins === navigator.plugins
I think Mozilla should be backwards compatible with NN4 so I am opening this 
bug.
Note that the fix of bug http://bugzilla.mozilla.org/show_bug.cgi?id=9392 seems 
to have caused this but I can't understand why the fix implemented a different 
behaviour in Mozilla than there is in NN4. document.plugins is not part of any 
standard so if Mozilla supports it it should be backwards compatible with NN4.

Comment 1

[Peter Lubczynski]

Confirming that this is still happening....

...sounds like a DOM bug

Comment 2

[Prashant Desale]

SEVERITY = LOW [(1)No Crash, (2)Good functional failure, (3)No Cosmetic failure]
VISIBILITY = HIGH [(1) real website usage could be high because of backward 
compatibility. (2)Gets one point of compatibility with other browsers since it 
works very well on NS4.x. (3)Gets one more point on compliance with adopted 
techonology which  is JS] 

PRIORITY = VISIBILITY * SEVERITY

Hence Priority = p3

adding word "qawanted" because I'm setting this priority on available data & if 
someone feels otherwise then please investigate this more & feel free to change 
this priority.

Comment 3

[Fabian Guisset]

Stealing this bug. Patch coming up.

Comment 4

[Fabian Guisset]

There is a problem... .plugins is part of nsIDOMNSDocument.idl, which means it's
implemented only by nsDocument. .embeds is part of nsIDOMNSHTMLDocument.idl,
which means it's implemented only by nsHTMLDocument. We have two solutions:
1) Move .plugins to be only available to html documents (my preferred solution)
2) Make each subclass of nsDocument implement the method. However I don't know
why documents other than HTML should need access to it, and they can always use
navigator.plugins
Thoughts?

Comment 5

[Johnny Stenback (:jst)]

Given that document.plugins should be equal to document.embeds it doesn't make
sense for document.plugins to be available on any other documents than HTML. I
believe that when document.plugins was implemented (incorrectly) the belief was
that since it was equal to navigator.plugins it made sense to put it on all
documents. I agree with Fabian, let's move it from nsIDOMNSDocument to
nsIDOMNSHTMLDocument.

Thanks Fabian for looking into this!

Comment 6

[Fabian Guisset]

Attachment: Proposed fix.

Comment 7

[Johnny Stenback (:jst)]

Comment on attachment 69475 [details] [diff] [review]
Proposed fix.

>+// Mapped to document.embeds for NS4 compatibility
>+NS_IMETHODIMP
>+nsHTMLDocument::GetPlugins(nsIDOMHTMLCollection** aPlugins)
>+{
>+  *aPlugins = nsnull;

You don't need this here since you're simply forwarding this call to
GetEmbeds(), GetEmebeds() is responsible for nulling out this parameter.

>+
>+  return GetEmbeds(aPlugins);
> }
> 

Other than that, this looks awesome. Thank you Fabian for the quick fix, sr=jst

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 69475 [details] [diff] [review]
Proposed fix.

whee!  comments in interfaces!	r=bzbarsky

Comment 9

[Arun Ranganathan]

i'm marking this nsbeta1 as an extra precaution so that it goes on the radar,
but thanks Fabian for fixing it :-)

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

a=roc+moz for 0.9.9

Comment 11

[Fabian Guisset]

Checked in the trunk, marking FIXED, thanks for the reviews :-)

Comment 12

[Fabian Guisset]

Oh, I'd just like to say a booh to the reviewers for not catching that I had
forgotten nsXULDocument.cpp in my diff. That almost broke the build :-P

Comment 13

[Johnny Stenback (:jst)]

Oops! :-)

Comment 14

[Prashant Desale]

Verified with builds 2002-04-12-10 on WinNT.