Bug 144373 - REGRESSION(181868): Windows Live SkyDrive cannot open an excel file
Summary: REGRESSION(181868): Windows Live SkyDrive cannot open an excel file
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-04-28 20:27 PDT by Joseph Pecoraro
Modified: 2015-04-29 11:48 PDT (History)
3 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (6.52 KB, patch)
2015-04-28 20:42 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2015-04-28 20:27:07 PDT
* SUMMARY
REGRESSION(181868): Windows Live SkyDrive cannot open an excel file.

* STEPS TO REPRODUCE
1. Open <https://skydrive.live.com>
2. Sign In
3. Open/Create an Excel document
  => never loads, infinitely stays on loading page

* NOTES
The page does stuff like:

(Ewa.js)
---------------------
    var w = window, wHTMLElementPrototype = w.HTMLElement.prototype;
    wHTMLElementPrototype.__defineGetter__("innerText", function() {
        try {
            return this.textContent
        } catch (d) {
            for (var b = "", a = 0, c = this.childNodes.length; a < c; a++)
                if (this.childNodes[a].nodeType == 3) b += this.childNodes[a].textContent;
            return b
        }
    });
    wHTMLElementPrototype.__defineSetter__("innerText", function(b) {
        var a = document.createTextNode(b);
        this.innerHTML = "";
        this.appendChild(a)
    });
---------------------

• Firefox: the properties don’t exist on HTMLElement.prototype, so they are created
• Chrome: the properties do exist and are configurable, so they are updated
• WebKit: the properties do exist and are NOT configurable
  - before r181868 we failed silently
  - after r181868 we throw an exception
Comment 1 Joseph Pecoraro 2015-04-28 20:28:16 PDT
Hmm, it seems Web IDL prefers that interface attributes be configurable by default:
http://www.w3.org/TR/WebIDL/#es-attributes

> The property has attributes
> { [[Get]]: G, [[Set]]: S, [[Enumerable]]: true, [[Configurable]]: configurable },
> where: configurable is false if the attribute was declared with the
> [Unforgeable] extended attribute and true otherwise;

Seems like the ultimate best path forward would be to:

  (1) Make IDL attributes configurable
  (2) Keep this exception when trying to reconfigure a non-configurable property.

But for now, lets just go back to failing silently.
Comment 2 Joseph Pecoraro 2015-04-28 20:28:33 PDT
<rdar://problem/20642043>
Comment 3 Joseph Pecoraro 2015-04-28 20:42:04 PDT
Created attachment 251920 [details]
[PATCH] Proposed Fix

=(. This will Reopen 142934 and in there I have a plan for hopefully re-enabling this warning.
Comment 4 WebKit Commit Bot 2015-04-29 00:54:51 PDT
Comment on attachment 251920 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 251920

Committed r183535: <http://trac.webkit.org/changeset/183535>
Comment 5 WebKit Commit Bot 2015-04-29 00:54:55 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Geoffrey Garen 2015-04-29 11:05:50 PDT
> • Firefox: the properties don’t exist on HTMLElement.prototype, so they are
> created

O'rly? Most DOM properties in Firefox are getters and setters in the prototype chain. Out of curiosity, where *does* Firefox define innerText?
Comment 7 Joseph Pecoraro 2015-04-29 11:48:12 PDT
(In reply to comment #6)
> > • Firefox: the properties don’t exist on HTMLElement.prototype, so they are
> > created
> 
> O'rly? Most DOM properties in Firefox are getters and setters in the
> prototype chain. Out of curiosity, where *does* Firefox define innerText?

Firefox doesn't appear to support "innerText" at all.