Bug 172703 – REGRESSION(r215946): Can't reference a table cell in Google spreadsheet

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Ryosuke Niwa]

Reproduction steps:
1. Go to drive.google.com
2. Create a new Google spreadsheet or open an existing one
3. Click on one of the cells and type in = and then click another cell to reference it.
4. Press enter.

Result:
Google spreadsheet shows "Unable to load file" as an error.

[Ryosuke Niwa]

Observed on STP30.

[Ryosuke Niwa]

In the console, I see the error:

CustomError: Error in protected function: a.item is not a function. (In 'a.item(0)', 'a.item' is undefined)

[Radar WebKit Bug Importer]

<rdar://problem/32458086>

[Ryosuke Niwa]

Bisected to
http://trac.webkit.org/log/trunk/?mode=follow_copy&rev=215947&stop_rev=215945

It must be https://trac.webkit.org/changeset/215946/webkit given the error:

CustomError: Error in protected function: a.item is not a function. (In 'a.item(0)', 'a.item' is undefined)

This change was probably not compatible with the Web. I bet there is thousands of other web pages that rely on .item(~) call.

[Chris Dumez]

Will look into this today.

[Chris Dumez]

Yes, ClientRectList had an item() method, which is missing from the new DOMRect[] type that is returned by getClientRects() in the spec. I'll looks into the Google spreadsheet code a little but it looks to me that the specification is not WebCompatible and we may need to reintroduce a DOMRectList type :(

[Chris Dumez]

For now, I am unable to reproduce with WebKit ToT. Will try some more.

[Chris Dumez]

Ok, got it:
function Epb(a) {
    Dpb(a);
    a = xpb(a).Pq().getClientRects();
    return {
        first: a.item(0),
        nV: a.item(a.length - 1)
    }
}

in https://docs.google.com/static/spreadsheets2/client/js/2594367115-ritz_waffle_i18n_core.js

[Chris Dumez]

(In reply to Chris Dumez from comment #8)
> Ok, got it:
> function Epb(a) {
>     Dpb(a);
>     a = xpb(a).Pq().getClientRects();
>     return {
>         first: a.item(0),
>         nV: a.item(a.length - 1)
>     }
> }
> 
> in
> https://docs.google.com/static/spreadsheets2/client/js/2594367115-
> ritz_waffle_i18n_core.js

Filed https://github.com/w3c/csswg-drafts/issues/1479 against the specification. Will now re-introduce a List type in WebKit to fix Google Speadsheet.

[Chris Dumez]

Created attachment 311527 [details]
WIP Patch

[Chris Dumez]

Created attachment 311529 [details]
WIP Patch

[Chris Dumez]

Created attachment 311532 [details]
WIP Patch

[Chris Dumez]

Created attachment 311533 [details]
WIP Patch

[Chris Dumez]

Created attachment 311539 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 311539 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=311539&action=review

> Source/WebCore/dom/DOMRectList.idl:33
> +] interface DOMRectList {
> +    readonly attribute unsigned long length;
> +    getter DOMRect item(unsigned long index);
> +};

Huh, this doesn't have an indexed getter!? No wonder Google docs is using item to access each item.

[Chris Dumez]

Comment on attachment 311539 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=311539&action=review

>> Source/WebCore/dom/DOMRectList.idl:33
>> +};
> 
> Huh, this doesn't have an indexed getter!? No wonder Google docs is using item to access each item.

This is an indexed getter:
getter DOMRect item(unsigned long index);

[Chris Dumez]

Comment on attachment 311539 [details]
Patch

https://trac.webkit.org/r217576