Open Bug 279958 Opened 19 years ago Updated 2 years ago

Autoscroll shouldn't be invoked on embedded objects in Firefox on Mac OS X

Categories

(Firefox :: General, defect)

PowerPC
macOS
defect

Tracking

()

People

(Reporter: smichaud, Unassigned)

References

()

Details

Attachments

(2 files, 2 obsolete files)

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b) Gecko/20050120 Firefox/1.0+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b) Gecko/20050120 Firefox/1.0+

As of the fix for bug 151249 (which started appearing in Firefox
nightlies as of 12-16-2004), autoscrolling now works in Firefox on OS
X (if you've turned it on in your Preferences) -- it's invoked by
clicking the middle mouse button.

But it's even invoked when you middle-click over an embedded object
such as the QuickTime plugin or a Java applet -- which can lead to
strange results if the embedded object also accepts mouse input.  Good
examples are the QuickTime movie trailers at
http://www.apple.com/trailers/ and the "High Voltage News" ticker (a
Java applet) at http://www.fantasyasylum.com/.

Clicking the middle mouse button on an embedded object shouldn't turn
on autoscrolling.  It doesn't do so in Firefox (1.0 or recent
nightlies) on Windows (at least with Java applets).



Reproducible: Always
This patch isn't Firefox-specific or OS-X-specific.  I don't think
that's a problem -- I don't believe allowing middle clicks on embedded
objects to activate autoscrolling is the correct behavior in any
environment.  In any case I did try to find a more narrowly targeted
fix, but wasn't able to.

I haven't figured out why Firefox behaves correctly (as I see it) on
Windows -- maybe when you middle-click on an embedded object in that
environment, the middle click event gets swallowed by the embedded
object, so that Firefox never sees it.
More applet/element fun.
Status: UNCONFIRMED → NEW
Ever confirmed: true
What about <object> elements?  But be careful not to break HTML objects...
How does Camino handle this (the attached fixed isn't relevant for embedors)?
(In reply to comment #3)

That's why I didn't add an exclusion for HTMLObjectElement objects
(along with the ones for HTMLEmbedElement and HTMLAppletElement
objects).

But I admit that I haven't tested my patch thoroughly ... and this is
the first time I've played with Chrome :-)

So why not add an exclusion for objects that have a null .contentDocument?
(In reply to comment #4)

As far as I can tell Camino doesn't support autoscrolling ... but
please tell me if I'm wrong.

If I'm right, Camino may very well not handle this.

(In reply to comment #6)

I don't (yet) know enough about Chrome to answer your question.

I know it doesn't, but i'm not sure the if the fix is relevant for 'just'
autoscroll.
(In reply to comment #8)
> (In reply to comment #6)
> 
> I don't (yet) know enough about Chrome to answer your question.

http://www.xulplanet.com/references/objref/HTMLObjectElement.html
That wasn't a question.  That was a review comment.  Sorry for not phrasing it
more directly.
(In reply to comment #9)

I've changed the "isAutoscrollBlocker" method, which seems only to be
called from the "mousedown" handler.  As best I can tell the
"mousedown" handler only deals with autoscrolling.

(In reply to comment #9)

Are you saying (in effect) that Camino developers need to start
worrying about this issue for when it does start supporting
autoscroll, and that it would need a different kind of fix?

No, my question, in other words, was "woudn't we need a more 'general' fix?".
It's too bad that Chrome and XUL can't take care of everything :-)

When I first grepped the Mozilla source for "autoscroll", I did expect
to be working at a much lower level.  I suspect a general fix will
have to be quite low-level, and won't be easy.

General fix for what, exactly?
> General fix for what, exactly?

I don't really know ... which I suppose is part of the problem :-)

But here's a suggestion:  Make Mozilla and family not process any
mouse events through its high-level handlers that happen over an
embedded object.  Mozilla-and-family would still have to send these
events to plugins (for example via nsIPluginInstance::handleEvent()),
but could otherwise leave them alone.

I confine my suggestion to mouse events -- it's too much more
difficult to know the "location" of, say, a keyboard event.

That suggestion is really not acceptable for a variety of reasons (including
being a flat-out violation of the DOM specification).
I've followed Boris Zbarsky's suggestion (in comment #6).

But I have some questions:

What are some examples of HTMLObjectElements that have a null
contentDocument?  Or for that matter of HTMLObjectElements of any
kind?

I've assumed that "HTMLEmbedElement" and "HTMLAppletElement" are thin
layers on top of HTML "embed" and "applet" objects.  Is that correct?
Attachment #172500 - Attachment is obsolete: true
(In reply to comment #18)
> That suggestion is really not acceptable for a variety of reasons (including
> being a flat-out violation of the DOM specification).

So, why is this bug mac-specific? What are we doing today on other platforms?
We call preventDefault() on various mouse events in a variety of circumstances
in nsObjectFrame.  Some of these are ifdefed differently for mac, which
apparently needs to do extra event processing and sometimes not preventDefault()
the events.

> What are some examples of HTMLObjectElements that have a null
> contentDocument?  Or for that matter of HTMLObjectElements of any
> kind?

The latter is any <object>.  The former is any <object> showing a plugin or
image (as opposed to an <object> showing an HTML page, say).

> I've assumed that "HTMLEmbedElement" and "HTMLAppletElement" are thin
> layers on top of HTML "embed" and "applet" objects.  Is that correct?

They're not "thin layers". They _are_ embed and applet objects.
> > What are some examples of HTMLObjectElements that have a null
> > contentDocument?  Or for that matter of HTMLObjectElements of any
> > kind?
>
> The latter is any <object>.  The former is any <object> showing a
> plugin or image (as opposed to an <object> showing an HTML page,
> say).

Then it should be possible to replace this part of my latest patch:

+   if (node instanceof HTMLAppletElement || node instanceof HTMLEmbedElement)
+     return true;
+
+   if (node instanceof HTMLObjectElement && node.contentDocument == null)
+     return true;
+

with this:

+   if (node instanceof HTMLObjectElement && node.contentDocument == null)
+     return true;
+

But when I tried it (with the Java applets at
http://gemal.dk/browserspy/java.html), autoscrolling was only
suppressed over the applet using an embed tag, not over the one using
an applet tag.

Is this a bug in the Mozilla-family's implementation of the DOM?

> Then it should be possible to replace this part of my latest patch:

Er... no.  What gave you that idea?
> > Then it should be possible to replace this part of my latest patch:
>
> Er... no.  What gave you that idea?

So, then, an HTMLObjectElement containing an HTMLEmbedElement has a
null contentDocument, but one containing an HTMLAppletElement doesn't
have a null contentDocument?  Are there (or might there be)
intermediate layers in the second case but not the first?

> So, then, an HTMLObjectElement containing an HTMLEmbedElement has a
> null contentDocument, but one containing an HTMLAppletElement doesn't
> have a null contentDocument?  

What it contains doesn't matter.  What matters is what it's showing.

So actually, just checking contentDocument is wrong, since an object showing its
replaced content would have a null contentDocument but you'd possibly want to
activate autoscroll depending on what that replaced content is.
I wonder if it wouldn't be better to go back to this:

+   if (node instanceof HTMLAppletElement || node instanceof HTMLEmbedElement)
+     return true;
+

Then we wouldn't be suppressing autoscroll above images (I guess).
But images usually (always?) aren't interactive, so maybe that's the
correct behavior.

Can you think of other important cases we'd miss if we just did this?

You'd miss plugins shown via <object>.  They'd start autoscrolling.
Argh.  I'm so used to using the HTML "embed" and "object" tags
together that I'd forgotten that an "embed" tag is really
non-standard, and that the "object" tag can be used without it.  So an
HTMLObjectElement is just whatever's inside an HTML "object" tag.

> So actually, just checking contentDocument is wrong, since an object
> showing its replaced content would have a null contentDocument but
> you'd possibly want to activate autoscroll depending on what that
> replaced content is.

Please point me in the general direction of some relevant sample code
(preferrably in the Mozilla source) ... or maybe I should just ask you
to edit the patch as you see fit :-)

> So an HTMLObjectElement is just whatever's inside an HTML "object" tag.

No, an HTMLObjectElement is the "object" tag itself.

> Please point me in the general direction of some relevant sample code

There isn't any that I'm aware of.

And I'm not particularly interested in patching this, though I'm willing to
spend time pointing out problems in proposed patches if people are willing to
listen to me.  ;)
Attached patch Fix for bug 279958, rev 3 (obsolete) — Splinter Review
Should something like this work for HTMLObjectElements with dynamic
content?
Attachment #172578 - Attachment is obsolete: true
That won't work for objects pointing to XHTML, SVG, application/xml, objects
that have no type attribute (which is optional), etc, etc.
Yes, the type attribute is optional.  But I figure that tags which
don't include it (or the codetype attribute) for non-text objects
should be considered broken.

I forgot about the codetype attribute.  In a while I'll post a patch
that corrects this ... after I've cooked up some dynamically changing
object tags to test against.

Can you (or anyone else on this bug) think of good, tricky URLs to
test against?

>Yes, the type attribute is optional.  But I figure that tags which
>don't include it (or the codetype attribute) for non-text objects
>should be considered broken.

why? it's just a hint. the server-sent type attribute is what's important.
(note: it's possible that mozilla does not implement this correctly...)
> it's possible that mozilla does not implement this correctly

Well, I just tried dropping the type attribute (for a Java applet)
from an EMBED tag inside an OBJECT tag ... and the applet stopped
loading.  With the type attribute it loads fine.

And here's another question:  I can't seem to get a plain OBJECT tag
to work with Mozilla (to load a Java applet).  It only works if I put
an EMBED tag inside the object tag (as Sun recommends at
http://java.sun.com/j2se/1.4.2/docs/guide/plugin/developer_guide/using_tags.html#ie-nav,
and which everyone seems to follow (at least for applets)).  What's up
with this?

> Well, I just tried dropping the type attribute (for a Java applet)
> from an EMBED tag inside an OBJECT tag ... 

But we're talking about type on objects, not type on embeds.

> I can't seem to get a plain OBJECT tag to work with Mozilla (to load a Java
> applet). 

Try not putting an ActiveX classid on it.
> > I can't seem to get a plain OBJECT tag to work with Mozilla (to
> > load a Java applet).
>
> Try not putting an ActiveX classid on it.

Got it working now.  The trick was to put a "data" attribute
(containing the same info as the applet-specific "code" attribute) and
a "type" attribute inside the OBJECT's start tag, and include at least
a "code" PARAM in the tag's "body":

<OBJECT type="application/x-java-applet" data="Applet.class" ... >
    <PARAM NAME="code" VALUE="Applet.class">
    ...
</OBJECT>

"data" and "type" didn't work as PARAMs.  The presence or absence of
the ActiveX "classid" seems to make no difference.

And once again the "type" attribute is necessary ... though this time
the error condition is different if it's absent -- instead of not
being loaded at all, a corrupt copy of the applet is loaded.

> a corrupt copy of the applet is loaded

I take that back.  Graphical corruption (including a slider) appears
on the screen where the applet should be, but the contents of the
Applet.class file is not being interpreted as an applet.

On a side note, Mozilla's "front-end" tests include a nice set of
plugin tests.  Glad I stumbled across it:

http://www.mozilla.org/quality/browser/front-end/testcases/plugins/

> The presence or absence of the ActiveX "classid" seems to make no
> difference.

Oops, wrong.  You do need to get rid of it.

Attached patch Fix for bug 279958, rev 4 (obsolete) — Splinter Review
> I forgot about the codetype attribute.  In a while I'll post a patch
> that corrects this ... after I've cooked up some dynamically
> changing object tags to test against.

I've now fixed this.  I also tested this patch against a simple case
of a swapping one OBJECT tag for another, and it worked fine.

I put a very simple OBJECT tag (<OBJECT data="textfile"> </OBJECT>)
inside the body of a DIV tag, and displayed a button (an INPUT tag
button that calls some JavaScript) to replace the contents of the DIV
tag (by changing its innerHTML) with another ("pure") OBJECT tag
containing a Java applet.

At first the DIV tag contained a scrollable (but not editable) display
of "textfile"'s contents.  Middle-clicking above it caused that
scrollable display (not the parent window's display) to autoscroll
(entirely the correct behavior, I think).  After pressing the button
this display was replaced by the Java applet.  Middle-clicking on it
had no effect -- as I had hoped and expected.

I actually tried to get Mozilla to change the contents of a single
OBJECT tag ... but I couldn't get that to work.  I found that I could
change the OBJECT tag's "data", "type" and "innerHTML", but doing so
had no effect on the display.  This didn't even work when I tried to
change one Java applet for another (by changing the "data" and
"innerHTML", but not the "type").

> > a corrupt copy of the applet is loaded
>
> I take that back.  Graphical corruption (including a slider) appears
> on the screen where the applet should be, but the contents of the
> Applet.class file is not being interpreted as an applet.

I've now figured this out.  From my results above, it's apparent that,
absent any "type" attribute, Mozilla defaults to interpreting an
OBJECT tag's "data" as plain text.
Attachment #172634 - Attachment is obsolete: true
> From my results above, it's apparent that, absent any "type" attribute, Mozilla
> defaults to interpreting an OBJECT tag's "data" as plain text.

That's not true.  It defaults to using the extension in the URI, and once some
bugs in the back end are fixed it will just use the HTTP header for the type (if
any; if loading from disk it will use the extension).

Again, there are non-text/* types that you want to be scrollable.  See comment 31.
> > From my results above, it's apparent that, absent any "type"
> > attribute, Mozilla defaults to interpreting an OBJECT tag's "data"
> > as plain text.
>
> That's not true.  It defaults to using the extension in the URI, and
> once some bugs in the back end are fixed it will just use the HTTP
> header for the type (if any; if loading from disk it will use the
> extension).

Well, there do seem to be some extensions (e.g. .class) that the
Mozilla-family interpret incorrectly as plain text (I wonder if this
happens with all extensions that they don't know about).  But Firefox
stopped loading my "textfile" when I removed its extension -- so its
presence does seem to be important.

> That won't work for objects pointing to XHTML, SVG, application/xml,
> objects that have no type attribute (which is optional), etc, etc.

> Again, there are non-text/* types that you want to be scrollable.
> See comment 31.

I didn't realize that you were saying these types should be
scrollable.  Let me hazard a guess -- all the non-text/* types which
need to be scrollable have something to do with xml.  In any case,
would be it possible to build a (small) list of such types, or devise
a type-name grepping strategy that would reliably pick out the types
that need to be scrollable?

The types that need to be scrollable are "anything Mozilla handles natively".  A
lot of these are XML, but there is also application/x-javascript, image/svg+xml,
possibly a few others.

Most importantly, I really don't think we should be adding such a list here,
since any addition to the core code to handle more types would break it.

Further, any  <object> that is including any type Mozilla doesn't handle at all
(natively or via plugin) should be scrollable, since then we'll show the
replacement content.

So we should be allowing scrolling if:  1)  There is a contentDocument or 2) if
the replaced content for the object is being rendered.

I'm not sure we have scriptable apis to determine the latter, but it may be
worth creating some.
> So we should be allowing scrolling if:  1) There is a
> contentDocument or 2) if the replaced content for the object is
> being rendered.
>
> I'm not sure we have scriptable apis to determine the latter, but it
> may be worth creating some.

Sounds reasonable to me.  In the meantime, could we block
autoscrolling over an HTMLObjectElement that has a null
contentDocument (as per your original suggestion, and as implemented
in my "rev 2" attachment 172578 [details] [diff] [review])?

Attachment #172578 - Attachment is obsolete: false
Attachment #172748 - Attachment is obsolete: true
> In the meantime, could we block autoscrolling over an
> HTMLObjectElement that has a null contentDocument (as per your
> original suggestion, and as implemented in my "rev 2" attachment
> 172578 [edit])?

Or we could just fall back to my first patch (attachment 172500 [details] [diff] [review]).  It
wouldn't stop autoscrolling in all the necessary cases, but it would
catch the vast majority -- almost all plugins are loaded using an
APPLET tag or an EMBED tag (either alone or inside an OBJECT tab).

Attachment #172500 - Attachment is obsolete: false
That's up to the UI owners of Firefox.
This bug is _really_ ugly on Mac OS X, so I'm requesting that it block
the Firefox 1.1 release.

Flags: blocking-aviary1.1?
Flags: blocking-aviary1.1? → blocking-aviary1.1+
Flags: blocking-aviary1.1+ → blocking-aviary1.1-
Assignee: bross2 → nobody
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: