MimeTypeArray should have unenumerable named properties per spec

RESOLVED FIXED in Firefox 53

Status

()

Core
DOM
RESOLVED FIXED
a year ago
3 months ago

People

(Reporter: bz, Assigned: bz)

Tracking

({addon-compat, dev-doc-complete, site-compat})

Trunk
mozilla51
addon-compat, dev-doc-complete, site-compat
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox49 wontfix, firefox50 wontfix, firefox51 wontfix, firefox52 wontfix, firefox53 fixed)

Details

(Whiteboard: btpp-fixlater)

Attachments

(1 attachment)

It's not clear to me why the spec says this, though.  They're certainly enumerable in Firefox...  Anne, do you know?

Changing this could include some compat pain, unfortunately.
Flags: needinfo?(annevk)
Depends on: 1270349

Comment 1

a year ago
You discussed this with Ian back in 2013: https://www.w3.org/Bugs/Public/show_bug.cgi?id=22600 (found through grepping through the spec commit log for unenumerable).
Flags: needinfo?(annevk)
Whiteboard: btpp-fixlater
Created attachment 8786587 [details] [diff] [review]
MimeTypeArray should have unenumerable named properties per spec

No tests, like bug 1270366, and for the same reasons: the tests would depend on plugins...
Attachment #8786587 - Flags: review?(bkelly)
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
Attachment #8786587 - Flags: review?(bkelly) → review+

Comment 3

9 months ago
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b319fc865ce5
MimeTypeArray should have unenumerable named properties per spec.  r=bkelly

Comment 4

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b319fc865ce5
Status: ASSIGNED → RESOLVED
Last Resolved: 9 months ago
status-firefox51: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
status-firefox49: affected → wontfix
status-firefox50: --- → affected

Updated

9 months ago
Keywords: addon-compat, dev-doc-needed, site-compat
Backed out per bug 757726 comment 161 for now, until we kill off plugins and can make the change more safely.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Comment 6

9 months ago
Backout by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6819c6bc8508
Backed out changeset b319fc865ce5 per discussion in bug 757726 (starting somewhere around bug 757726 comment 157).

Updated

9 months ago
Blocks: 757726
Depends on: 1269807
Depends on: 1303093
Depends on: 1303076

Comment 7

8 months ago
backoutbugherder
https://hg.mozilla.org/mozilla-central/rev/6819c6bc8508
Status: REOPENED → RESOLVED
Last Resolved: 9 months ago8 months ago
Resolution: --- → FIXED
No, the whole point is this is not fixed...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Updated

8 months ago
status-firefox50: affected → wontfix
status-firefox51: fixed → wontfix
Seems like I should only reland this once the pref from bug 1269807 goes away (and in particular, should not land this for esr52), right?  Is there a bug tracking that pref being removed?
Flags: needinfo?(benjamin)
Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2016/navigator-plugins-and-mimetypes-will-be-unenumerable/
I don't think this change will be happening for 52.  See comment 9.
Flags: needinfo?(kohei.yoshino)
I guess this comes with 53, right? So the doc's version is "Future" for now.
Flags: needinfo?(kohei.yoshino)
> I guess this comes with 53, right?

So I assume.
What is the practical effect of this bug? Does this make navigator.mimeTypes not enumerable at all? Or is still possible to enumerate the *indexed* properties but not the named ones?

I filed bug 1308761 to track the ESR52-specific setting. There is not a specific bug for removing the rest of this goop in 53 yet, but I don't think that should block you.
Flags: needinfo?(benjamin)
> Or is still possible to enumerate the *indexed* properties but not the named ones?

The practical effect is that a for-in loop on navigator.mimeTypes will only see the indexed properties and will not see the named ones.  Object.getOwnPropertyNames will still see both.

I guess my question is whether I should land this now and we disable it on the ESR52 branch or whether I should just wait until m-c becomes 53 before landing.
Flags: needinfo?(benjamin)
This should wait for 53 please.

The behavior of Chrome is here is surprising to me, in that it enumerates the indexed properties as well as "length", "item", and "namedItem". I hope that this change won't affect web compat in the following manner:

var foundFlash = false;
for (var k in navigator.mimeTypes) {
  if (k == "application/x-shockwave-flash") {
    foundFlash = true;
  }
}

Because people do a lot of weird stuff on the internet.

Is this a valuable change to make? Could we get away with not making any change here at all? Spec be damned and being risk-averse on purpose.
Flags: needinfo?(benjamin)
> This should wait for 53 please.

Will do.

> The behavior of Chrome is here is surprising to me, in that it enumerates the indexed
> properties as well as "length", "item", and "namedItem".

That would be our behavior after this change too, fwiw.

> I hope that this change won't affect web compat in the following manner

That loop sets foundFlash to false in at least Chrome and Safari, right?  I haven't tested Edge recently because I don't have access to Edge in an environment where Flash is installed.

> Is this a valuable change to make?

Not technically, but it's useful politically in terms of with getting other browsers to make other changes to align with specs and us.

> Could we get away with not making any change here at all?

We could, but if we did that I would prefer to get the spec changed accordingly and hence have to convince the other UAs to follow us...
Flags: needinfo?(bzbarsky)
Flags: needinfo?(benjamin)
We *know* that there's all kinds of webcompat issues here. I don't think we should ask Chrome to change their current behavior, because that might cause webcompat issues for them. I just don't see much value in changing our current behavior either. Until/unless we see webcompat issues because we're not doing the same thing as Chrome, the safer thing is to make as few changes to this legacy system as possible.
Flags: needinfo?(benjamin)
OK, but that puts us in a pretty sad position where the spec is meaningless and costs both us and the WHATWG/W3C  credibility.  :(

I will wait for 52 to branch to Aurora and then carefully check exactly what other browsers do here, I guess.  We could always opt for having the spec explicitly say that both behaviors are allowed if we think that minimizes risk....
OK.  I have double-checked and all of Edge, Safari, Chrome agree here.  We're the only ones that disagree.

I'm going to go ahead and land this after 52 branches.

Comment 21

6 months ago
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3a28c54656b3
MimeTypeArray should have unenumerable named properties per spec.  r=bkelly
Flags: needinfo?(bzbarsky)
status-firefox52: --- → wontfix
status-firefox53: --- → affected

Comment 22

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/3a28c54656b3
Status: REOPENED → RESOLVED
Last Resolved: 8 months ago6 months ago
status-firefox53: affected → fixed
Resolution: --- → FIXED
I've added notes to:

https://developer.mozilla.org/en-US/docs/Web/API/NavigatorPlugins/mimeTypes
https://developer.mozilla.org/en-US/Firefox/Releases/53#DOM_HTML_DOM

(We never actually created a page for MimeTypeArray)
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.