Closed Bug 959992 Opened 10 years ago Closed 9 years ago

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

Categories

(Core :: DOM: Core & HTML, defect)

Product:
Core
Component:
DOM: Core & HTML
Version:
26 Branch
Platform:
x86_64
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla42
Tracking Flags:
Tracking Status
firefox42 --- fixed

People

(Reporter: nickolay8, Assigned: bzbarsky)

Details

(Keywords: dev-doc-complete, regression, site-compat)

Attachments

(2 files)

img_global.html
744 bytes, text/html
Details
Go back to not treating properties that the named properties object exposes as enumerable
9.56 KB, patch
peterv
: review+
Details | Diff | Splinter Review
Attached file 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
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
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.
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.
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
Closed: 9 years ago
status-firefox42: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: