Bug 143678 - javascript window.open window size
Summary: javascript window.open window size
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac OS X 10.10
: P2 Major
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-13 13:50 PDT by Josef
Modified: 2017-09-18 03:27 PDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Josef 2015-04-13 13:50:34 PDT
Environment: on MacBook with Retina display with and without external monitor.
If I am open with Javascript a new window with e.g.
window.open("","queryLogFrame","height=700,width=1000,scrollbars=no,resizable,status");
The window height is about 30-50 pixel smaller than with Firefox or Chrome.
This problem I notice the first time with 10.10 in conjunction with the Retina display.
--
Josef
Comment 1 Simon Pieters (:zcorpan) 2017-04-12 06:23:11 PDT
This causes failures in these tests:

https://github.com/w3c/web-platform-tests/pull/5390

I think it would be good to have predictable behavior here, that "height" feature sets the viewport height like in other browsers.
Comment 2 Chris Dumez 2017-04-28 20:05:38 PDT
Hmm, we do:
FloatSize viewportSize = page->chrome().pageRect().size();
FloatRect windowRect = page->chrome().windowRect();
windowRect.setHeight(*features.height + (windowRect.height() - viewportSize.height()));

So we *seem* to be doing the right thing. However, viewportSize and windowRect are identical :/
Comment 3 Chris Dumez 2017-04-28 20:30:39 PDT
(In reply to Chris Dumez from comment #2)
> Hmm, we do:
> FloatSize viewportSize = page->chrome().pageRect().size();
> FloatRect windowRect = page->chrome().windowRect();
> windowRect.setHeight(*features.height + (windowRect.height() -
> viewportSize.height()));
> 
> So we *seem* to be doing the right thing. However, viewportSize and
> windowRect are identical :/

From what I can see viewportSize is wrong and windowRect is right.
Comment 4 Chris Dumez 2017-05-01 16:32:35 PDT
Tim says we should subtract the topContentInset because the view is the same height as the window since Yosemite.
Comment 5 Tim Horton 2017-05-01 16:36:59 PDT
Really I said "probably we should leave it up to the WebKits and ask them for window-size-for-given-view-size or something, instead of hoping that subtracting will give you what you want, but if you want to quickly work around the problem you could subtract topContentInset" :)
Comment 7 Chris Dumez 2017-05-01 18:31:57 PDT
Page::setTopContentInset() gets called *after* we set the new Window size unfortunately.
Comment 8 Tim Horton 2017-05-01 18:35:58 PDT
(In reply to Chris Dumez from comment #7)
> Page::setTopContentInset() gets called *after* we set the new Window size
> unfortunately.

Where is the size set? You might be able to reorder it, because TCI is sent in WebProcessCreationParameters, so it's not like we don't have it.
Comment 9 Tim Horton 2017-05-01 18:36:20 PDT
(In reply to Tim Horton from comment #8)
> (In reply to Chris Dumez from comment #7)
> > Page::setTopContentInset() gets called *after* we set the new Window size
> > unfortunately.
> 
> Where is the size set? You might be able to reorder it, because TCI is sent
> in WebProcessCreationParameters, so it's not like we don't have it.

WebPageCreationParameters, whatever.
Comment 10 Chris Dumez 2017-05-01 18:38:39 PDT
(In reply to Tim Horton from comment #9)
> (In reply to Tim Horton from comment #8)
> > (In reply to Chris Dumez from comment #7)
> > > Page::setTopContentInset() gets called *after* we set the new Window size
> > > unfortunately.
> > 
> > Where is the size set? You might be able to reorder it, because TCI is sent
> > in WebProcessCreationParameters, so it's not like we don't have it.
> 
> WebPageCreationParameters, whatever.

The logic for setting the new window size is in:
RefPtr<Frame> createWindow(Frame& openerFrame, Frame& lookupFrame, const FrameLoadRequest& request, const WindowFeatures& features, bool& created) inside FrameLoader.cpp.

We first call chrome().createWindow() to create the Window.
Then we compute the height we want.
Finally we call chrome().setWindowRect().
Comment 11 Chris Dumez 2017-05-01 19:07:38 PDT
(In reply to Chris Dumez from comment #10)
> (In reply to Tim Horton from comment #9)
> > (In reply to Tim Horton from comment #8)
> > > (In reply to Chris Dumez from comment #7)
> > > > Page::setTopContentInset() gets called *after* we set the new Window size
> > > > unfortunately.
> > > 
> > > Where is the size set? You might be able to reorder it, because TCI is sent
> > > in WebProcessCreationParameters, so it's not like we don't have it.
> > 
> > WebPageCreationParameters, whatever.
> 
> The logic for setting the new window size is in:
> RefPtr<Frame> createWindow(Frame& openerFrame, Frame& lookupFrame, const
> FrameLoadRequest& request, const WindowFeatures& features, bool& created)
> inside FrameLoader.cpp.
> 
> We first call chrome().createWindow() to create the Window.
> Then we compute the height we want.
> Finally we call chrome().setWindowRect().

And to make things a bit more complicated, between the window creation and setting the window size, there is a call to chrome().setToolbarsVisible(), which I believe would impact the topContentInset.