Bug 24215 – Sites using gears broken in r41209, by display:none on <object> (24215)

• Component: Plug-ins
• Status: RESOLVED FIXED

[Matt Perry]

This change (http://trac.webkit.org/changeset/41209) breaks sites using Gears.  Such sites have a snippet of javascript that loads gears by appending an object tag into the page, with display set to "none".  Now that that behavior has changed, object tags with "display=none" don't load the corresponding NPAPI plugins, and Gears no longer works.

It would be nice to have a workaround for Gears until sites have time to change.

[Matt Perry]

Alternatively, if the plugin were still loaded, but just not displayed, that would work for gears as well.

[Simon Fraser (smfr)]

Fixing display:none to work for <object> was bug 15081.

[Geoffrey Garen]

visibility:hidden is the "right" way to hide gears in CSS, while still asking it to load.

Do we know where these Gears sites get their code snippets from? Maybe we should evangelize the source of the snippets.

[Geoffrey Garen]

Also, can you list some example sites?

[Matt Perry]

The sites get their snippets from http://code.google.com/apis/gears/tools.html#gears_init .  We can update this source, but will also have to get sites to update their copies.

Some gears-using sites that I'm aware of:
- http://mail.google.com
- http://docs.google.com
- http://rememberthemilk.com
- http://zoho.com
- http://www.passpack.com
- http://www.mindmeister.com

There's probably only a handful or two of others.

[Eric Seidel (no email)]

Google Latitude also uses Gears, although maybe that doesn't require the plugin to load on the page.

[Matt Perry]

We now have a workaround for Chrome so that Gears will still work.  I'm not sure how easy a similar workaround for Safari would be.

[Geoffrey Garen]

Would you be willing to explain what the work-around is?

[Geoffrey Garen]

<rdar://problem/6633049>

[Matt Perry]

I'll post a link when I check in the change, but the basic idea is that we run a snippet of javascript on each page which adds a javascript getter (using __defineGetter__) where the gears object is expected.  The first time a page accesses this getter, the gears object is injected in a manner similar to how gears_init.js does it (except without setting display="none").

[Geoffrey Garen]

I don't think there's anything about that solution that can't be written in WebKit. Is there a particular reason that you chose to patch Chrome instead of WebKit?

Another good solution for WebKit, if you're interested in improving WebKit, is to detect a type attribute of "application/x-googlegears" on an object element, in HTMLObjectElement::rendererIsNeeded, and apply special behavior based on that, to return true despite a display:none style.

[Matt Perry]

The feeling on the Gears team is that this "workaround" for Chrome is a better way of loading Gears into pages, since it doesn't rely on gears_init.js scripts in the wild.  We're hoping to find a similar method for Safari.  If that's not possible in the short term, we can add a temporary special-case to webkit.

For those interested, the Chrome change is here: http://src.chromium.org/viewvc/chrome?view=rev&revision=10663

[Jeremy Moskovich]

Created attachment 28142 [details]
Special case gears object tag

Per comment #11, this patch checks the type attribute and instantiates the plugin in the case of display:none and Gears.
Is there a good way to test this?

[Matt Perry]

Here's what the Gears team would like to happen:
- Commit temporary special-case fix to webkit (see Jeremy's patch).  (Though we'd need to test that it works in Safari first!)
- Gears team works on a solution (in gears itself) so that we no longer rely on gears_init.js at all.
- Remove the temporary special-case in webkit.

How does that sound?

[Jeremy Moskovich]

I've verified that the attached patch fixes the issue in Safari.

[Simon Fraser (smfr)]

I'm ok with the patch as long as you file a follow-up bug to remove the hack once Gears is fixed, and the comment in the code refers to that bug.

[Jeremy Moskovich]

Created attachment 28260 [details]
Updated patch

Per Simon's comment, point to a followup bug for removing the temporary fix.

[Simon Fraser (smfr)]

Wait, bug 24356 does not exist. Please fix the bug number in the comment.

[Matt Perry]

Created attachment 28275 [details]
above patch with correct bug number

I just fixed the above patch with the correct bug number (bug 24346).

[Mark Rowe (bdash)]

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

[Darin Fisher (:fishd, Google)]

Comment on attachment 28275 [details]
above patch with correct bug number

Landed as http://trac.webkit.org/changeset/41473

[Pam Greene (IRC:pamg)]

Landed, so closing.