Firefox 26 creates enumerable properties on window for ids of <img> tags

RESOLVED FIXED in Firefox 42

Status

()

Core
DOM: Core & HTML
RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: Nickolay Platonov, Assigned: bz)

Tracking

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

26 Branch
mozilla42
x86_64
Linux
dev-doc-complete, regression, site-compat
Points:
---

Firefox Tracking Flags

(firefox42 fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
Created attachment 8360297 [details]
img_global.html

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:26.0) Gecko/20100101 Firefox/26.0 (Beta/Release)
Build ID: 20131206152142

Steps to reproduce:

Open the attached file in FF 26 with console opened.


Actual results:

See the output:
DIV : true
IMG : true
has DIV false
has IMG true

Starting from Firefox 26, browser creates enumerables in "window" for ids of the images on the page. It works correctly in older releases


Expected results:

The output from previous releases:
DIV : true
IMG : true
has DIV false
has IMG false

Updated

4 years ago
Component: Untriaged → JavaScript Engine
Product: Firefox → Core
The relevant specs are at http://www.whatwg.org/specs/web-apps/current-work/multipage/browsers.html#named-access-on-the-window-object and http://heycam.github.io/webidl/#property-enumeration and per those specs the correct output is "has DIV true" and "has IMG true".

Right now we return false for the former because we only ask the document for its supported names in WindowNamedPropertiesHandler::getOwnPropertyNames, which is a subset of the supported names on Window (what changed in Firefox 26 was returning the document supported names here).  So unless the spec changes, either we should mark this invalid and file a new one on the ids of all elements not being enumerated or we should morph this bug to cover that issue.

One other note: with the move of the window IDL properties to the object itself (as the spec calls for) this testcase causes infinite recursion and never actually runs the testcase...
Component: JavaScript Engine → DOM: Core & HTML
For that last bit, in shipping Safari the testcase does:

  [Error] RangeError: Maximum call stack size exceeded.
Summary: Firefox 26 creates an enumerbale properties for ids of <img> tags → Firefox 26 creates enumerable properties on window for ids of <img> tags
(Reporter)

Comment 3

3 years ago
Indeed, and the same exception in Firefox. Works fine in Chrome. Can someone please explain how such simple test case can cause a "maximum call stack size exceeded" exception? It looks totally fine.
Sure.  It's quite simple.  Per spec, the testcase is exactly identical to this testcase:

  <script>
    window.onload = function() { onload(); }
  </script>

because the "load" event handler for <body> is hoisted to the window and the "onload" IDL property is an own property of the window (so the "var onload = ...." bit actually assigned to the IDL property).  So you first set window.onload to that function in the testcase, then set it to the function that just calls onload().

It's working in Chrome at the moment, because in Chrome right now the .onload IDL property is on Window.prototype not on the window object and hence the var shadows it.  That's not what the spec says to do, and they're working on fixing it.
(Reporter)

Comment 5

3 years ago
Ah, so its because of the "onload" name of the handler? "onLoad" or something different should work fine, right? I see. Thanks for explanations!
> Ah, so its because of the "onload" name of the handler?

Yes, it's because the function is just calling itself.  Giving the thing you want to call a name other than "onload" should work fine.
OK, so I think the spec should get changed here.  I filed https://www.w3.org/Bugs/Public/show_bug.cgi?id=28390 on that happening.  For now I'm just going to switch us back to our old behavior, which matches Safari and Chrome.
Created attachment 8587653 [details] [diff] [review]
Go back to not treating properties that the named properties object exposes as enumerable
Attachment #8587653 - Flags: review?(peterv)
Assignee: nobody → bzbarsky
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 8587653 [details] [diff] [review]
Go back to not treating properties that the named properties object exposes as enumerable

Review of attachment 8587653 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/base/test/test_window_element_enumeration.html
@@ +63,5 @@
> +    is(names2.indexOf("four"), -1,
> +       "<div> with name should not be in our enumeration list");
> +    is(names3.indexOf("four"), -1,
> +       "<div> with name should not be in GSP own prop list");
> +    

Trailing whitespace.
Attachment #8587653 - Flags: review?(peterv) → review+
https://hg.mozilla.org/mozilla-central/rev/fe4679e180ca
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42

Updated

2 years ago
Keywords: dev-doc-needed, regression, site-compat
Added a comment in https://developer.mozilla.org/en-US/Firefox/Releases/42#DOM_HTML_DOM
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.