Last Comment Bug 741266 - document.open with 3 or more arguments should call OpenJS, not Open
: document.open with 3 or more arguments should call OpenJS, not Open
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: mozilla14
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-01 19:19 PDT by Boris Zbarsky [:bz]
Modified: 2012-04-09 10:11 PDT (History)
3 users (show)
bzbarsky: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Testcase (248 bytes, text/html)
2012-04-01 19:19 PDT, Boris Zbarsky [:bz]
no flags Details
document.open with 3 or more arguments should invoke the scriptable version of window.open, not the noscript one. (3.70 KB, patch)
2012-04-01 19:37 PDT, Boris Zbarsky [:bz]
jst: review+
Details | Diff | Splinter Review

Description Boris Zbarsky [:bz] 2012-04-01 19:19:02 PDT
Created attachment 611338 [details]
Testcase

Right now it seems to call Open(), which makes the behavior wrong when opening a sized window.  Try the attached testcase.

Changing to OpenJS here makes things work right.  Since we really only support document.open() from script, that seems like a reasonable change to me.
Comment 1 Boris Zbarsky [:bz] 2012-04-01 19:37:22 PDT
Created attachment 611339 [details] [diff] [review]
document.open with 3 or more arguments should invoke the scriptable version of window.open, not the noscript one.
Comment 2 :Ms2ger (⌚ UTC+1/+2) 2012-04-02 08:35:20 PDT
Comment on attachment 611339 [details] [diff] [review]
document.open with 3 or more arguments should invoke the scriptable version of window.open, not the noscript one.

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

::: content/html/document/test/test_bug741266.html
@@ +20,5 @@
> +
> +/** Test for Bug 741266 **/
> +var w = window.open("", "", "width=100,height=100");
> +is(w.innerHeight, 100, "Popup height should be 100 when opened with window.open");
> +is(w.innerWidth, 100, "Popup widthshould be 100 when opened with window.open");

Nit: space in the message (twice)
Comment 3 Boris Zbarsky [:bz] 2012-04-06 14:14:13 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/d83c1ffb7397 with that nit fixed.
Comment 4 Marco Bonardo [::mak] (Away 6-20 Aug) 2012-04-06 16:35:47 PDT
backed out cause the newly added test fails on Windows
https://hg.mozilla.org/integration/mozilla-inbound/rev/3be23dcd43e4

87069 ERROR TEST-UNEXPECTED-FAIL | /tests/content/html/document/test/test_bug741266.html | Popup width should be 100 when opened with window.open - got 114, expected 100
87071 ERROR TEST-UNEXPECTED-FAIL | /tests/content/html/document/test/test_bug741266.html | Popup width should be 100 when opened with document.open - got 114, expected 100
Comment 5 Boris Zbarsky [:bz] 2012-04-06 18:05:25 PDT
Bah.  That's totally broken...

I'll rework the test a bit.
Comment 6 Boris Zbarsky [:bz] 2012-04-06 18:23:37 PDT
With the fixed test: https://hg.mozilla.org/integration/mozilla-inbound/rev/a7ae9d421848
Comment 7 Matt Brubeck (:mbrubeck) 2012-04-09 10:11:25 PDT
https://hg.mozilla.org/mozilla-central/rev/a7ae9d421848

Note You need to log in before you can comment on or make changes to this bug.