[webkit-changes] cvs commit: WebKit/WebCoreSupport.subproj
WebBridge.m
Geoffrey
ggaren at opensource.apple.com
Tue Dec 20 09:06:05 PST 2005
ggaren 05/12/20 09:06:04
Modified: . ChangeLog
khtml/ecma kjs_window.cpp
kwq KWQKHTMLPart.mm KWQKHTMLPartBrowserExtension.mm
KWQKPartsBrowserExtension.h WebCoreBridge.h
. ChangeLog
WebCoreSupport.subproj WebBridge.m
Added: manual-tests window-open-features.html
manual-tests/resources 200x200.png popup200x200.html
Log:
WebCore:
Reviewed by John.
Fixed <rdar://problem/4310363> JavaScript window.open: Height is 1
pixel short, and related bugs.
There were a few bugs here.
(1) Our code took size arguments and applied them to the window's
content rect. That's incorrect. The Rhino book says the arguments
should apply to the WebView. Other things that occupy the content
rect include the tab bar, the status bar, and the 1 pixel border
between brushed metal and document. All of these used to impinge
on the web page's display area.
The fix is to calculate sizing based on the WebView instead of
the content rect. This means that the webViewContentRect and
setContentRect delegate methods are obsolete and no longer called
by any of our code. (setContentRect was never called in the
first place.)
(2) None of our sizing accounted for scaled resolutions.
The fix is to ask the WebView to scale all coordintes for us.
(3) Our code assumed that all window accoutrements were on by default.
Safari works that way, but other WebKit clients might not.
The fix is always to explicitly set an on/off state.
(a) To facilitate scaling, I added a new bridge method, webView, to
access the webView.
(b) For internal consistency, I changed ___Bars to ___bars in bridge
methods, and ___bars to ___Bars in WinArgs data members. (Interestingly,
the different classes in our code are evenly divided on which format to
use.)
Added manual test:
* manual-tests/window-open-features.html: Added.
* manual-tests/resources/200x200.png: Added.
* manual-tests/resources/popup200x200.html: Added.
* khtml/ecma/kjs_window.cpp:
(KJS::showModalDialog): see (b)
(KJS::WindowFunc::callAsFunction): see (b)
* kwq/KWQKHTMLPart.mm:
(KWQKHTMLPart::statusbarVisible): see (b)
* kwq/KWQKHTMLPartBrowserExtension.mm:
(KHTMLPartBrowserExtension::createNewWindow):
At the top of this method, I just did some formatting cleanup and
moved the 'referrer' variable closer to where it's used.
The changes in the middle of the method are (3), the bottom, (2).
* kwq/KWQKPartsBrowserExtension.h:
(KParts::WindowArgs::WindowArgs): see (b)
* kwq/WebCoreBridge.h: see (a)
WebKit:
Reviewed by John.
Part of fix for <rdar://problem/4310363> JavaScript window.open: Height
is 1 pixel short, and related bugs. See WebCore ChageLog.
* WebCoreSupport.subproj/WebBridge.m:
(-[WebBridge webView]): Added method.
Revision Changes Path
1.10 +57 -0 WebCore/ChangeLog
Index: ChangeLog
===================================================================
RCS file: /cvs/root/WebCore/ChangeLog,v
retrieving revision 1.9
retrieving revision 1.10
diff -u -r1.9 -r1.10
--- ChangeLog 20 Dec 2005 09:30:23 -0000 1.9
+++ ChangeLog 20 Dec 2005 17:05:50 -0000 1.10
@@ -1,3 +1,60 @@
+2005-12-20 Geoffrey Garen <ggaren at apple.com>
+
+ Reviewed by John.
+
+ Fixed <rdar://problem/4310363> JavaScript window.open: Height is 1
+ pixel short, and related bugs.
+
+ There were a few bugs here.
+ (1) Our code took size arguments and applied them to the window's
+ content rect. That's incorrect. The Rhino book says the arguments
+ should apply to the WebView. Other things that occupy the content
+ rect include the tab bar, the status bar, and the 1 pixel border
+ between brushed metal and document. All of these used to impinge
+ on the web page's display area.
+
+ The fix is to calculate sizing based on the WebView instead of
+ the content rect. This means that the webViewContentRect and
+ setContentRect delegate methods are obsolete and no longer called
+ by any of our code. (setContentRect was never called in the
+ first place.)
+
+ (2) None of our sizing accounted for scaled resolutions.
+
+ The fix is to ask the WebView to scale all coordintes for us.
+
+ (3) Our code assumed that all window accoutrements were on by default.
+ Safari works that way, but other WebKit clients might not.
+
+ The fix is always to explicitly set an on/off state.
+
+ (a) To facilitate scaling, I added a new bridge method, webView, to
+ access the webView.
+
+ (b) For internal consistency, I changed ___Bars to ___bars in bridge
+ methods, and ___bars to ___Bars in WinArgs data members. (Interestingly,
+ the different classes in our code are evenly divided on which format to
+ use.)
+
+ Added manual test:
+ * manual-tests/window-open-features.html: Added.
+ * manual-tests/resources/200x200.png: Added.
+ * manual-tests/resources/popup200x200.html: Added.
+
+ * khtml/ecma/kjs_window.cpp:
+ (KJS::showModalDialog): see (b)
+ (KJS::WindowFunc::callAsFunction): see (b)
+ * kwq/KWQKHTMLPart.mm:
+ (KWQKHTMLPart::statusbarVisible): see (b)
+ * kwq/KWQKHTMLPartBrowserExtension.mm:
+ (KHTMLPartBrowserExtension::createNewWindow):
+ At the top of this method, I just did some formatting cleanup and
+ moved the 'referrer' variable closer to where it's used.
+ The changes in the middle of the method are (3), the bottom, (2).
+ * kwq/KWQKPartsBrowserExtension.h:
+ (KParts::WindowArgs::WindowArgs): see (b)
+ * kwq/WebCoreBridge.h: see (a)
+
2005-12-20 Eric Seidel <eseidel at apple.com>
Reviewed by mjs.
1.204 +3 -3 WebCore/khtml/ecma/kjs_window.cpp
Index: kjs_window.cpp
===================================================================
RCS file: /cvs/root/WebCore/khtml/ecma/kjs_window.cpp,v
retrieving revision 1.203
retrieving revision 1.204
diff -u -r1.203 -r1.204
--- kjs_window.cpp 19 Dec 2005 19:52:47 -0000 1.203
+++ kjs_window.cpp 20 Dec 2005 17:05:51 -0000 1.204
@@ -642,7 +642,7 @@
wargs.dialog = true;
wargs.resizable = boolFeature(features, "resizable");
- wargs.scrollbarsVisible = boolFeature(features, "scroll", true);
+ wargs.scrollBarsVisible = boolFeature(features, "scroll", true);
wargs.statusBarVisible = boolFeature(features, "status", !trusted);
wargs.toolBarsVisible = false;
@@ -1491,7 +1491,7 @@
winargs.menuBarVisible = false;
winargs.toolBarsVisible = false;
winargs.statusBarVisible = false;
- winargs.scrollbarsVisible = false;
+ winargs.scrollBarsVisible = false;
winargs.resizable = false;
QStringList flist = QStringList::split(',', features);
QStringList::ConstIterator it = flist.begin();
@@ -1575,7 +1575,7 @@
else if (key == "fullscreen")
winargs.fullscreen = (val == "1" || val == "yes");
else if (key == "scrollbars")
- winargs.scrollbarsVisible = !(val == "0" || val == "no");
+ winargs.scrollBarsVisible = !(val == "0" || val == "no");
}
}
1.707 +1 -1 WebCore/kwq/KWQKHTMLPart.mm
Index: KWQKHTMLPart.mm
===================================================================
RCS file: /cvs/root/WebCore/kwq/KWQKHTMLPart.mm,v
retrieving revision 1.706
retrieving revision 1.707
diff -u -r1.706 -r1.707
--- KWQKHTMLPart.mm 19 Dec 2005 19:52:57 -0000 1.706
+++ KWQKHTMLPart.mm 20 Dec 2005 17:05:52 -0000 1.707
@@ -1837,7 +1837,7 @@
bool KWQKHTMLPart::statusbarVisible()
{
- return [_bridge isStatusBarVisible];
+ return [_bridge isStatusbarVisible];
}
bool KWQKHTMLPart::toolbarVisible()
1.52 +70 -69 WebCore/kwq/KWQKHTMLPartBrowserExtension.mm
Index: KWQKHTMLPartBrowserExtension.mm
===================================================================
RCS file: /cvs/root/WebCore/kwq/KWQKHTMLPartBrowserExtension.mm,v
retrieving revision 1.51
retrieving revision 1.52
diff -u -r1.51 -r1.52
--- KWQKHTMLPartBrowserExtension.mm 3 Oct 2005 21:13:05 -0000 1.51
+++ KWQKHTMLPartBrowserExtension.mm 20 Dec 2005 17:05:53 -0000 1.52
@@ -71,96 +71,97 @@
{
KWQ_BLOCK_EXCEPTIONS;
- NSString *frameName = urlArgs.frameName.length() == 0 ? nil : urlArgs.frameName.getNSString();
-
- WebCoreBridge *bridge;
-
- NSString *referrer;
- QString argsReferrer = urlArgs.metaData()["referrer"];
- if (argsReferrer.length() > 0) {
- referrer = argsReferrer.getNSString();
- } else {
- referrer = [_part->bridge() referrer];
- }
-
ASSERT(!winArgs.dialog || urlArgs.frameName.isEmpty());
if (partResult)
*partResult = NULL;
- if (frameName != nil) {
- bridge = [_part->bridge() findFrameNamed:frameName];
- if (bridge != nil) {
- if (!url.isEmpty()) {
- [bridge loadURL:url.getNSURL() referrer:referrer reload:urlArgs.reload userGesture:true target:nil triggeringEvent:nil form:nil formValues:nil];
- }
- [bridge focusWindow];
- if (partResult) {
- *partResult = [bridge part];
- }
- return;
- }
+ NSString *frameName = urlArgs.frameName.length() == 0 ? nil : urlArgs.frameName.getNSString();
+ if (frameName) {
+ // FIXME: Can't you just use _part->findFrame?
+ if (WebCoreBridge *bridge = [_part->bridge() findFrameNamed:frameName]) {
+ if (!url.isEmpty()) {
+ QString argsReferrer = urlArgs.metaData()["referrer"];
+ NSString *referrer;
+ if (argsReferrer.length() > 0)
+ referrer = argsReferrer.getNSString();
+ else
+ referrer = [_part->bridge() referrer];
+
+ [bridge loadURL:url.getNSURL()
+ referrer:referrer
+ reload:urlArgs.reload
+ userGesture:true
+ target:nil
+ triggeringEvent:nil
+ form:nil
+ formValues:nil];
+ }
+
+ [bridge focusWindow];
+
+ if (partResult)
+ *partResult = [bridge part];
+
+ return;
+ }
}
+ WebCoreBridge *bridge;
if (winArgs.dialog)
bridge = [_part->bridge() createModalDialogWithURL:url.getNSURL()];
else
bridge = [_part->bridge() createWindowWithURL:url.getNSURL() frameName:frameName];
+
if (!bridge)
return;
- if (!winArgs.toolBarsVisible) {
- [bridge setToolbarsVisible:NO];
- }
-
- if (!winArgs.statusBarVisible) {
- [bridge setStatusBarVisible:NO];
- }
+ if ([bridge part])
+ [bridge part]->setName(urlArgs.frameName);
- if (!winArgs.scrollbarsVisible) {
- [bridge setScrollbarsVisible:NO];
- }
+ if (partResult)
+ *partResult = [bridge part];
- if (!winArgs.resizable) {
- [bridge setWindowIsResizable:NO];
+ [bridge setToolbarsVisible:winArgs.toolBarsVisible];
+ [bridge setStatusbarVisible:winArgs.statusBarVisible];
+ [bridge setScrollbarsVisible:winArgs.scrollBarsVisible];
+ [bridge setWindowIsResizable:winArgs.resizable];
+
+ NSRect windowFrame = [bridge windowFrame];
+
+ NSSize scaleRect = [[bridge webView] convertSize:NSMakeSize(1, 1) toView:nil];
+ float scaleX = scaleRect.width;
+ float scaleY = scaleRect.height;
+
+ // In JavaScript, the coordinate system of the window is the same as the coordinate system
+ // of the WebView, so we translate 'left' and 'top' from WebView coordinates to window coordinates.
+ // (If the screen resolution is scaled, window coordinates won't match WebView coordinates.)
+ if (winArgs.xSet)
+ windowFrame.origin.x = winArgs.x * scaleX;
+ if (winArgs.ySet) {
+ // In JavaScript, (0, 0) is the top left corner of the screen.
+ float screenTop = NSMaxY([[[NSScreen screens] objectAtIndex:0] frame]);
+ windowFrame.origin.y = screenTop - (winArgs.y * scaleY) - windowFrame.size.height;
+ }
+
+ // 'width' and 'height' specify the dimensions of the WebView, but we can only resize the window,
+ // so we compute a delta, translate it to window coordinates, and use the translated delta
+ // to resize the window.
+ NSRect webViewFrame = [[bridge webView] frame];
+ if (winArgs.widthSet) {
+ float widthDelta = (winArgs.width - webViewFrame.size.width) * scaleX;
+ windowFrame.size.width += widthDelta;
+ }
+ if (winArgs.heightSet) {
+ float heightDelta = (winArgs.height - webViewFrame.size.height) * scaleY;
+ windowFrame.size.height += heightDelta;
+ windowFrame.origin.y -= heightDelta;
}
- if (winArgs.xSet || winArgs.ySet || winArgs.widthSet || winArgs.heightSet) {
- NSRect frame = [bridge windowFrame];
- NSRect contentRect = [bridge windowContentRect];
-
- if (winArgs.xSet) {
- frame.origin.x = winArgs.x;
- }
-
- if (winArgs.ySet) {
- float heightForFlip = NSMaxY([[[NSScreen screens] objectAtIndex:0] frame]);
- frame.origin.y = heightForFlip - (winArgs.y + frame.size.height);
- }
-
- if (winArgs.widthSet) {
- frame.size.width += winArgs.width - contentRect.size.width;
- }
-
- if (winArgs.heightSet) {
- float heightDelta = winArgs.height - contentRect.size.height;
- frame.size.height += heightDelta;
- frame.origin.y -= heightDelta;
- }
-
- [bridge setWindowFrame:frame];
- }
+ [bridge setWindowFrame:windowFrame];
[bridge showWindow];
-
- if ([bridge part]) {
- [bridge part]->setName(urlArgs.frameName);
- }
- if (partResult) {
- *partResult = [bridge part];
- }
-
KWQ_UNBLOCK_EXCEPTIONS;
}
1.28 +2 -2 WebCore/kwq/KWQKPartsBrowserExtension.h
Index: KWQKPartsBrowserExtension.h
===================================================================
RCS file: /cvs/root/WebCore/kwq/KWQKPartsBrowserExtension.h,v
retrieving revision 1.27
retrieving revision 1.28
diff -u -r1.27 -r1.28
--- KWQKPartsBrowserExtension.h 4 Dec 2005 00:32:34 -0000 1.27
+++ KWQKPartsBrowserExtension.h 20 Dec 2005 17:05:53 -0000 1.28
@@ -81,7 +81,7 @@
bool menuBarVisible;
bool statusBarVisible;
bool toolBarsVisible;
- bool scrollbarsVisible;
+ bool scrollBarsVisible;
bool resizable;
bool fullscreen;
bool xSet;
@@ -91,7 +91,7 @@
bool dialog;
WindowArgs() : x(0), y(0), width(0), height(0),
- menuBarVisible(true), statusBarVisible(true), toolBarsVisible(true), scrollbarsVisible(true),
+ menuBarVisible(true), statusBarVisible(true), toolBarsVisible(true), scrollBarsVisible(true),
resizable(true), fullscreen(false),
xSet(false), ySet(false), widthSet(false), heightSet(false),
dialog(false)
1.355 +4 -2 WebCore/kwq/WebCoreBridge.h
Index: WebCoreBridge.h
===================================================================
RCS file: /cvs/root/WebCore/kwq/WebCoreBridge.h,v
retrieving revision 1.354
retrieving revision 1.355
diff -u -r1.354 -r1.355
--- WebCoreBridge.h 10 Dec 2005 20:40:18 -0000 1.354
+++ WebCoreBridge.h 20 Dec 2005 17:05:53 -0000 1.355
@@ -28,6 +28,7 @@
#import <JavaScriptCore/npruntime.h>
#import <JavaVM/jni.h>
#import <WebCore/WebCoreKeyboardAccess.h>
+#import <WebKit/WebView.h>
#ifdef __cplusplus
@@ -473,6 +474,7 @@
- (NSString *)generateFrameName;
- (void)frameDetached;
- (NSView *)documentView;
+- (WebView *)webView;
- (void)loadURL:(NSURL *)URL referrer:(NSString *)referrer reload:(BOOL)reload userGesture:(BOOL)forUser target:(NSString *)target triggeringEvent:(NSEvent *)event form:(DOMElement *)form formValues:(NSDictionary *)values;
- (void)postWithURL:(NSURL *)URL referrer:(NSString *)referrer target:(NSString *)target data:(NSArray *)data contentType:(NSString *)contentType triggeringEvent:(NSEvent *)event form:(DOMElement *)form formValues:(NSDictionary *)values;
@@ -500,8 +502,8 @@
- (BOOL)areToolbarsVisible;
- (void)setToolbarsVisible:(BOOL)visible;
-- (BOOL)isStatusBarVisible;
-- (void)setStatusBarVisible:(BOOL)visible;
+- (BOOL)isStatusbarVisible;
+- (void)setStatusbarVisible:(BOOL)visible;
- (BOOL)areScrollbarsVisible;
- (void)setScrollbarsVisible:(BOOL)visible;
- (NSWindow *)window;
1.1 WebCore/manual-tests/window-open-features.html
Index: window-open-features.html
===================================================================
<html>
<head>
<script>
var w;
function test(sFeatures)
{
if (w && !w.closed)
w.close();
w = window.open("popup200x200.html", "popup", sFeatures);
}
</script>
</head>
<body>
<p>This test checks our support for the 'features' argument to window.open.</p>
<p>Each button below will open a new window. Check to make sure that the window has the specified attributes.</p>
<p style="color:red">NOTE: Please verify that the behavior in this test is resolution independent.</p>
<p>To test a non-standard resolution:</p>
<ol>
<li>Open Quartz Debug (/Developer/Applications/Performance Tools).</li>
<li>Select Tools -> Show User Interface Resolution.</li>
<li>Set the resolution to 2.0.</li>
<li>Restart Safari.</li>
</ol>
<hr>
<p>Window size (no toolbars): You should see a red 1 pixel border along every edge of this page, and no scrollbars.</p>
<input type="button" value="open it!" onclick='test("width=200, height=200, scrollbars=yes")';
<hr>
<p>Window size (all toolbars): You should see a red 1 pixel border along every edge of this page, and no scrollbars.</p>
<input type="button" value="open it!" onclick='test("width=200, height=200, scrollbars=yes, menubar=yes, status=yes, toolbar=yes")';
<hr>
<p>Window positioning: This window should be aligned exactly to the top right corner of the screen.</p>
<p style="color:red">NOTE: This won't work with resolution 2.0 until we fix screen.width.</p>
<input type="button" value="open it!" onclick='test("width=200,height=200,left=" + (screen.width - 200) + ",screenY=0")';
<hr>
</body>
</html>
1.1 WebCore/manual-tests/resources/200x200.png
<<Binary file>>
1.1 WebCore/manual-tests/resources/popup200x200.html
Index: popup200x200.html
===================================================================
<html>
<head>
<title>Popup 100x100</title>
</head>
<body bgcolor="#ffffff" leftmargin="0" topmargin="0" marginheight="0" marginwidth="0">
<img src="200x200.png" width="200" height="200">
</body>
</html>
1.3415 +10 -0 WebKit/ChangeLog
Index: ChangeLog
===================================================================
RCS file: /cvs/root/WebKit/ChangeLog,v
retrieving revision 1.3414
retrieving revision 1.3415
diff -u -r1.3414 -r1.3415
--- ChangeLog 20 Dec 2005 10:07:53 -0000 1.3414
+++ ChangeLog 20 Dec 2005 17:05:56 -0000 1.3415
@@ -1,3 +1,13 @@
+2005-12-20 Geoffrey Garen <ggaren at apple.com>
+
+ Reviewed by John.
+
+ Part of fix for <rdar://problem/4310363> JavaScript window.open: Height
+ is 1 pixel short, and related bugs. See WebCore ChageLog.
+
+ * WebCoreSupport.subproj/WebBridge.m:
+ (-[WebBridge webView]): Added method.
+
2005-12-20 Eric Seidel <eseidel at apple.com>
Reviewed by mjs.
1.378 +6 -0 WebKit/WebCoreSupport.subproj/WebBridge.m
Index: WebBridge.m
===================================================================
RCS file: /cvs/root/WebKit/WebCoreSupport.subproj/WebBridge.m,v
retrieving revision 1.377
retrieving revision 1.378
diff -u -r1.377 -r1.378
--- WebBridge.m 20 Dec 2005 09:59:11 -0000 1.377
+++ WebBridge.m 20 Dec 2005 17:06:04 -0000 1.378
@@ -195,6 +195,12 @@
return [[_frame findFrameNamed:name] _bridge];
}
+- (WebView *)webView
+{
+ ASSERT(_frame != nil);
+ return [_frame webView];
+}
+
- (NSView *)documentView
{
ASSERT(_frame != nil);
More information about the webkit-changes
mailing list