Bug 24215

Summary: Sites using gears broken in r41209, by display:none on <object> (24215)
Product: WebKit Reporter: Matt Perry <mpComplete>
Component: Plug-insAssignee: Nobody <webkit-unassigned>
Severity: Normal CC: mrowe, playmobil, shadow2531, simon.fraser, stefmanevski, zwarich
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Description Flags
Special case gears object tag
Updated patch
simon.fraser: review+
above patch with correct bug number simon.fraser: review+

Description Matt Perry 2009-02-26 16:52:22 PST
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.
Comment 1 Matt Perry 2009-02-26 17:04:31 PST
Alternatively, if the plugin were still loaded, but just not displayed, that would work for gears as well.
Comment 2 Simon Fraser (smfr) 2009-02-26 17:06:12 PST
Fixing display:none to work for <object> was bug 15081.
Comment 3 Geoffrey Garen 2009-02-26 17:15:23 PST
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.
Comment 4 Geoffrey Garen 2009-02-26 17:15:56 PST
Also, can you list some example sites?
Comment 5 Matt Perry 2009-02-26 17:37:05 PST
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.
Comment 6 Eric Seidel (no email) 2009-02-26 17:47:43 PST
Google Latitude also uses Gears, although maybe that doesn't require the plugin to load on the page.
Comment 7 Matt Perry 2009-02-27 15:06:26 PST
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.
Comment 8 Geoffrey Garen 2009-02-27 15:57:10 PST
Would you be willing to explain what the work-around is?
Comment 9 Geoffrey Garen 2009-02-27 16:10:13 PST
Comment 10 Matt Perry 2009-02-27 16:15:59 PST
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").
Comment 11 Geoffrey Garen 2009-02-27 16:21:38 PST
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.
Comment 12 Matt Perry 2009-02-27 17:28:03 PST
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
Comment 13 Jeremy Moskovich 2009-03-01 06:14:29 PST
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?
Comment 14 Matt Perry 2009-03-03 11:47:14 PST
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?
Comment 15 Jeremy Moskovich 2009-03-03 12:59:16 PST
I've verified that the attached patch fixes the issue in Safari.
Comment 16 Simon Fraser (smfr) 2009-03-03 13:14:20 PST
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.
Comment 17 Jeremy Moskovich 2009-03-04 02:07:13 PST
Created attachment 28260 [details]
Updated patch

Per Simon's comment, point to a followup bug for removing the temporary fix.
Comment 18 Simon Fraser (smfr) 2009-03-04 09:07:57 PST
Wait, bug 24356 does not exist. Please fix the bug number in the comment.
Comment 19 Matt Perry 2009-03-04 11:02:39 PST
Created attachment 28275 [details]
above patch with correct bug number

I just fixed the above patch with the correct bug number (bug 24346).
Comment 20 Mark Rowe (bdash) 2009-03-05 20:35:27 PST
*** Bug 24383 has been marked as a duplicate of this bug. ***
Comment 21 Darin Fisher (:fishd, Google) 2009-03-05 21:16:22 PST
Comment on attachment 28275 [details]
above patch with correct bug number

Landed as http://trac.webkit.org/changeset/41473
Comment 22 Pam Greene (IRC:pamg) 2009-03-11 11:03:33 PDT
Landed, so closing.