Bug 16128 - ignore resizable=no for window.open()
: ignore resizable=no for window.open()
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: HTML DOM
: 528+ (Nightly build)
: Macintosh Mac OS X 10.4
: P2 Enhancement
Assigned To: Nobody
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-11-25 00:34 PST by rahul abrol
Modified: 2007-12-01 16:34 PST (History)
0 users

See Also:


Attachments
patch v1 (3.46 KB, patch)
2007-11-25 00:35 PST, rahul abrol
aroben: review-
Details | Formatted Diff | Diff
patch v1.1 (4.38 KB, patch)
2007-11-25 12:50 PST, rahul abrol
aroben: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description rahul abrol 2007-11-25 00:34:07 PST
i see no reason to honor this key.  the current code is following win IE's implementation, but neither firefox nor opera support this.  see, e.g.

https://bugzilla.mozilla.org/show_bug.cgi?id=177838
Comment 1 rahul abrol 2007-11-25 00:35:50 PST
Created attachment 17502 [details]
patch v1
Comment 2 Adam Roben (:aroben) 2007-11-25 00:52:18 PST
Comment on attachment 17502 [details]
patch v1

+    // Ignore resizable key
+    //    else if (keyString == "resizable")
+    //        windowFeatures.resizable = value;

We don't like to leave commented-out code in our source tree. Having a comment about ignoring resizable is good (perhaps at the start of this if/else chain), but the code should just be removed.

+    // Default to true
+    windowFeatures.resizable = true;

This comment should be omitted, as it add any information to the code below it.

Should we just remove windowFeatures.resizable entirely?

r- so the above issues can be fixed.
Comment 3 rahul abrol 2007-11-25 12:50:35 PST
Created attachment 17512 [details]
patch v1.1
Comment 4 Adam Roben (:aroben) 2007-11-25 20:58:36 PST
Comment on attachment 17512 [details]
patch v1.1

-     The IE rule is: all features except for channelmode and fullscreen default to YES, but
-     if the user specifies a feature string, all features default to NO. (There is no public
-     standard that applies to this method.)
-     
+     The IE rule is: all features except for channelmode and fullscreen default to YES, but if the user specifies a feature string, all features default to NO. (There is no public standard that applies to this method.)

I think the newlines here should be restored.

r=me, but whoever lands this should fix the above issue.
Comment 5 Mark Rowe (bdash) 2007-12-01 08:42:37 PST
Landed in r28300.
Comment 6 Geoffrey Garen 2007-12-01 15:43:38 PST
The change to window-open-features-parsing.html was incorrect. If you want to change the expected result, fine. But you can't change the parsed string -- the whole point of the test is to see how the browser parses the string as a whole.
Comment 7 Geoffrey Garen 2007-12-01 15:59:37 PST
Fixed in revision 28310.
Comment 8 Geoffrey Garen 2007-12-01 16:34:51 PST
Now that I think about it more, we probably shouldn't have made this change in WebKit at all. WebKit has a responsibility to report the data fed to window.open to its client. It's up to the client to decide upon a behavior. If we want windows in Safari always to be resizable, then Safari should implement that policy, not WebKit.