Back out bug 932322 on beta until we're sure the spec will stay as it is

RESOLVED WONTFIX

Status

()

defect
RESOLVED WONTFIX
5 years ago
4 months ago

People

(Reporter: bzbarsky, Assigned: bzbarsky)

Tracking

(Depends on 1 bug, {dev-doc-complete})

unspecified
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox28+ fixed, firefox29+ fixed, firefox30+ fixed, firefox31+ wontfix, firefox32+ wontfix, firefox33 wontfix, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed, b2g-v2.0 wontfix, b2g-v2.1 wontfix)

Details

(Whiteboard: [qa-])

Attachments

(1 attachment, 3 obsolete attachments)

Or something?  In any case, for now it's breaking sites and we don't know what final behavior we'll want.

I have no idea how to properly track this so we keep remembering to do it on every train.  :(
Blocks: 973702
OS: Mac OS X → All
Hardware: x86 → All
I propose to just backout everywhere until the spec is saner. It introduced more problems than it fixed.
The problems if fixes are the ones that will come up when we start removing prefixed crud from window.  So of course it's not fixing any right this second.

If you have a concrete proposal for how to fix the spec, do feel free to post to public-script-coord.

In the meantime, I think there is value in leaving this on nightly/aurora so we can find out whether there are other compat issues.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
And to be clear, the only concrete counterproposals to the current spec are:

1)  Standardize some of the prefixed props.
2)  arv proposal of "let's just break sites if we ever remove prefixed props and then
    evangelize them to fix themselves".

In practice, #2 will act like #1, since no one will remove prefixed stuff if that means breaking sites.
Why we can't do more similar things to Chrome and IE? They aren't breaking either sites.
Attachment #8381897 - Attachment is obsolete: true
> Why we can't do more similar things to Chrome and IE?

Chrome has some combination of never removing prefixed properties and putting some properties (but not all!) on the object itself.

IE has a behavior that requires never removing prefixed properties.
And the point is, if we want to spec the "never remove prefixed properties" behavior, the spec also needs to spec a prefixed property that everyone should implement.  Otherwise it's useless for new UAs.
Attachment #8382233 - Flags: review?(jst) → review+
Comment on attachment 8382233 [details] [diff] [review]
Mostly back out bug 932322 for now; only define the unforgeable properties on the window object itself.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 932322
User impact if declined: Some sites are likely broken
Testing completed (on m-c, etc.): Passes tests
Risk to taking this patch (and alternatives if risky): Some risk, but this is a
   pretty straight backout, so should get us back more or less to where we used
   to be in Firefox 27...
String or IDL/UUID changes made by this patch:  None.

Note that I want to only back this out on beta for now...
Attachment #8382233 - Flags: approval-mozilla-beta?
Boris, before approving the uplift, can you tell me what you are planning to do with regard to 29 & 30? Thanks
Flags: needinfo?(bzbarsky)
Keep gathering compat info and seeing whether we can get Chrome to agree on what the spec should be.  We may end up landing this patch on 29 and 30 as well in the future, if needed...
Flags: needinfo?(bzbarsky)
Comment on attachment 8382233 [details] [diff] [review]
Mostly back out bug 932322 for now; only define the unforgeable properties on the window object itself.

Good plan, approving for today's beta.
Attachment #8382233 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Whiteboard: [qa-]
What's the plan for 29 Beta and 30 Aurora?
Unclear so far.  Depends on what happens with the spec.
Boris, beta 4 should be released today. Do you think we should consider a backout also in 29? Thanks
Flags: needinfo?(bzbarsky)
Yes, we should.
Attachment #8382233 - Attachment is obsolete: true
Comment on attachment 8400045 [details] [diff] [review]
Version of the patch for beta 29

Try run at https://tbpl.mozilla.org/?tree=Try&rev=8f2d8576af48
Attachment #8400045 - Flags: approval-mozilla-beta?
Flags: needinfo?(bzbarsky)
Attachment #8400045 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Note that we've discussed this with the Apple and Google folks and they agree the spec should stay as is.
Comment on attachment 8400045 [details] [diff] [review]
Version of the patch for beta 29

Time to do this again, for 30, until we get the opener issues in bug 989584 sorted out.
Attachment #8400045 - Flags: approval-mozilla-beta+ → approval-mozilla-beta?
Attachment #8400045 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8400045 - Attachment is obsolete: true
Yeah, browser_bug982298.js is busted.  It's importing Timer.jsm into the browser window!

I removed that gunk, and did a try push at https://tbpl.mozilla.org/?tree=Try&rev=f901a087c1ab
Depends on: 1013999
Lawrence, could you help here? (webcompat perspective)
Flags: needinfo?(lmandel)
Note that there are also sites that depend on us NOT landing this patch.  See bug 1024806.

I think we're done here.  We have no plans to check this in again for 31.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Depends on: 1024806
Resolution: --- → WONTFIX
Okay, then I'll document Bug 932322 in the Firefox 31 site compat doc. The original note that was in the 28 compat doc is below. Boris, can you please refine this?

--

<h3>Global variables with the same name as <code>window</code> properties are no longer accessible</h3>

<p>A typical impact of this change has been reported as <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=943958" title="Bug 943958 - Global variables with the same name as window properties can't accessed">Bug&nbsp;943958</a>. Code like <code>var name = 1</code> no longer works as expected, instead <code>name</code> will return a string <code>'1'</code> because of the {{ domxref("window.name") }} property. Web developers should avoid using global variables with the same name as {{ domxref("window") }} properties.</p>
Flags: needinfo?(bzbarsky)
I would phrase it like so:

 <h3>Global variables with the same name as <code>window</code> properties now call the property setter when they are set.</h3>

The paragraph after that point is OK, though maybe it's worth pointing out that this is also the behavior WebKit and Blink have for window properties.
Flags: needinfo?(bzbarsky)
I take it comment 32 means that we're going ahead with this spec based change in 31. Do we know the expected breakage that we expect on the Web due to this change? Hallvord and Seif should be able to run their crawler to detect usage to give us some idea of what this change will mean for Web content when it hits release.
Flags: needinfo?(lmandel) → needinfo?(hsteen)
> I take it comment 32 means that we're going ahead with this spec based change in 31.

Yes, correct.

> Do we know the expected breakage that we expect on the Web due to this change?

We don't expect much, for what it's worth.  We've been shipping this in nightlies and aurora and partway through beta in 28, 29, 30, and 31, and in nightly and partway through aurora in 32.  We've found and fixed a few issues along the way, so it's not like we're just trying this with no testing at all...

Of course replacing guesswork with data is always appreciated.  ;)
(In reply to Boris Zbarsky [:bz] from comment #37)
> Of course replacing guesswork with data is always appreciated.  ;)

Hallvord - Given the description in comment 33 and comment 34 I don't know how easy it will be to scrape the Web for usage and possible breakage. I suppose that we can scan for global declaration of all window properties. Thoughts?
Depends on: 1026113
Depends on: 1024899
(In reply to Lawrence Mandel [:lmandel] from comment #38)
> (In reply to Boris Zbarsky [:bz] from comment #37)
> > Of course replacing guesswork with data is always appreciated.  ;)
> 
> Hallvord - Given the description in comment 33 and comment 34 I don't know
> how easy it will be to scrape the Web for usage and possible breakage. I
> suppose that we can scan for global declaration of all window properties.

In this case, gathering the data from JS might be possible for many (not all) properties - it might be more robust and cover more properties if we put some logging in the ES engine itself I guess - but the real challenge would lie in the interpretation of the data once collected. If we've found 10 000 sites setting window.name, how many of those set it on purpose, and how many set it by mistake? I don't think that can be automated.
Flags: needinfo?(hsteen)
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.