Bug 13467 – REGRESSION (r20972): Google Spreadsheets: spreadsheet controls not displayed

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Tommi Komulainen]

I know Google says Spreadsheets does not support Safari yet, but it used to work just fine with r20961.

With r21003 when loading a spreadsheet, the controls (toolbars etc.) disappear and only the spreadsheet cells (iframe?) are shown in the window. The cells fill the whole window and keyboard navigation no longer works between cells, but instead scrolls the whole window.

Regressed in <http://trac.webkit.org/projects/webkit/changeset/20972> (fix for <rdar://problem/4671964>). Rolling out that patch fixes this bug.

[Antti Koivisto]

This was broken by r20972 which removed document property from iframe element. This was done to fix a bug with SAP application. The new behavior should match Firefox.

The problem is that after this change the spreadsheet grid gets loaded to main view instead of the iframe it should go to.

Looking around the obfuscated code in http://spreadsheets.google.com/client/js/2100283068-trix_main.js I saw the following function:

  ff[_P].hj = function ()
  {
    if ($b)
      {
        if (this.Ui[hb] == null)
          {
            return null;
          }
        return this.Ui[hb][lg];
      }
    else
      {
        return this.Ui[jg];
      }
  };

Here $b is a Safari test result, hb="document", lg="defaultView" and jg="contentWindow" and this.Ui is an iframe element (I think). Basically for Safari it does "iframe.document.defaultView" whereas for all other browsers it does "iframe.contentWindow". With iframe.document gone, this would not work anymore. I can't verify that this exact function causes the problem (there are other Safari tests and other places where iframe content is accessed in various ways) but it certainly looks like something which would fail after the change in r20972.

[W. Andy Carrel]

Nice detective work, thanks! I've made contact with an appropriate engineer.

[Antti Koivisto]

I tested that at least in simple cases iframe.contentWindow works fine in ToT, so the special code path for Safari should no be needed. Please tell if you find there is some more subtle brokeness that still needs special handling so we can fix it.

[Darin Adler]

<rdar://problem/5183687>

[Sam Weinig]

I can no longer test this as I can no longer get a spreadsheet up at all.  I am getting this assert while loading:

ASSERTION FAILED: startPosition.isNotNull() && endPosition.isNotNull()
(rendering/RenderTextControl.cpp:433 void WebCore::RenderTextControl::setSelectionRange(int, int))

(In reply to comment #6)
> I can no longer test this as I can no longer get a spreadsheet up at all.  I am
> getting this assert while loading:
> 
> ASSERTION FAILED: startPosition.isNotNull() && endPosition.isNotNull()
> (rendering/RenderTextControl.cpp:433 void
> WebCore::RenderTextControl::setSelectionRange(int, int))
> 

Apply the patch from bug 13413 :)

[Justin Garcia]

Looks OK with Safari 3 Beta.