196563 - View > Page Source disable if document contains unsupported plug in (flash)

Status: RESOLVED FIXED

(Core Graveyard :: View Source, defect)

Description

[Chris Casciano]

I've recently removed the flash player from my systems. Since doing so I've
noticed that the view source is disabled on any page that contains a flash
object. "Page Source" item in the View menu and context menus are disabled and
CMD/CTRL-U doesn't do anything. this happens on every page i've visited that
include flash content including: http://www.wddg.com/

This behavior holds true for framed sites (can't view source of any doc) that
have flash content in just one of the frames a such as: http://www.joshuadavis.com/

Note: I've seen the view source disabled on rare occation in the past but a
reload of the document always fixed it. This isn't the case here. Reload doesn't
change anything.

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.3) Gecko/20030302
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.3) Gecko/20030308

Comment 1

[Clinton Nicholls^3]

Confirmation: I browsed to a site that had Java elements, and my machine doesn't
have the Java plugin installed. I tried to do a view source for some reason(I
don't know why), and I got an error saying it couldn't view source because of an
unsupported element. This is true.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Looks like we set isImage to be disabled for such sites...  Is this due to the
onLocationChange or onStateChange code in nsBrowserStatusHandler.js ?

Comment 3

[Chris Casciano]

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

Comment 4

[Boris Zbarsky [:bzbarsky]]

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

Comment 5

[R.K.Aa.]

I have flash player enabled and working, yet the menus at CNN.com fail on
sub-pages. Main page is OK. Bug 191612.

Comment 6

[[e-mail]]

It's not the onLocationChange code, because if you switch tabs and back then the
view source enables correctly, so it must be the onStateChange code. Is there a
good reason why this sort of thing is done in onStateChange?

Comment 7

[Boris Zbarsky [:bzbarsky]]

Yeah... because for normal page transitions it's impossible to get the right
content-type in onLocationChange -- the aRequest is null.  Further, for a
full-page plugin, content.document.contentType will not work (will be
undefined).  So if you want to disable menuitems for those we need to do it like
this.... maybe.

Comment 8

[[e-mail]]

FYI I see both the onLocationChange enabling and the onStateChange disabling
with that particular test case.

Comment 9

[[e-mail]]

Ah, I see now, the onLocationChange fires on the old document :-/

Comment 10

[Boris Zbarsky [:bzbarsky]]

So.. comment 7 just got obsoleted by bug 90256.  Plugins now get a document.

jst, are there still valid cases when we would not have a document in the
content area?

Comment 11

[[e-mail]]

So the fix now is to update using the "top" document rather than the document
whose state is changing?

Comment 12

[[e-mail]]

Attachment: Proposed patch

Comment 13

[rbs]

Comment on attachment 118552 [details] [diff] [review]
Proposed patch

sr=rbs, patch looks good now that bug 90256 always gives a doc to full page
plug-ins. Plus, your test |if (content.doc)| is ample protection.

Comment 14

[Boris Zbarsky [:bzbarsky]]

To neil.  Thanks for fixing this!

Comment 15

[Asa Dotzler [:asa]]

Comment on attachment 118552 [details] [diff] [review]
Proposed patch

please land first thing in beta. thanks.

Comment 16

[[e-mail]]

Fix checked in.