New issue
Advanced search Search tips

Issue 823588 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

hterm: ctrl+shift+N doesn't open new window with same height

Reported by johnl...@gmail.com, Mar 20 2018

Issue description

UserAgent: Mozilla/5.0 (X11; CrOS x86_64 10032.76.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.150 Safari/537.36
Platform: 61.4.54 (Developer Build - neverware) developer-build chromeover64

Steps to reproduce the problem:
1. Connect to some server with Secure Shell
2. Open a new window with ctrl+shift+N

What is the expected behavior?
The new window should have the same size of the old window.

What went wrong?
The height of the new window is smaller than the old window.

WebStore page: https://chrome.google.com/webstore/detail/secure-shell/pnhechapfaindjhompbnflcldabbghjo

Did this work before? N/A 

Chrome version: 63.0.3239.150  Channel: n/a
OS Version: 10032.76.0
Flash Version: 

I found that this functionality is implemented in the function "onCtrlN_". In its first commit, the height is specified as "window.outerHeight", which was correct. However, it was removed by https://code.google.com/archive/p/chromium-os/issues/34123 and changed to "window.innerHeight" by https://code.google.com/archive/p/chromium-os/issues/38272. I think the former is a workaround, and the later is just a mistake. I will submit a patch to change it back to "window.outerHeight".
 
old-window.png
200 KB View Download
new-window.png
126 KB View Download

Comment 2 Deleted

Comment 3 Deleted

after the tracker migration, the new bugs are  issue 216271  and  issue 220421  respectively.

this sounds like a bug in Chrome.  the MDN documentation (i'll pull out the relevant pieces below) says hterm is using the correct fields.  the current code is:
    window.open(document.location.href, '',
                'chrome=no,close=yes,resize=yes,scrollbars=yes,' +
                'minimizable=yes,width=' + window.innerWidth +
                ',height=' + window.innerHeight);

window.open: https://developer.mozilla.org/en-US/docs/Web/API/Window/open
  height: Specifies the height of the content area, viewing area of the new secondary window in pixels. The height value includes the height of the horizontal scrollbar if present.
  width: Specifies the width of the content area, viewing area of the new secondary window in pixels. The width value includes the width of the vertical scrollbar if present. The width value does not include the sidebar if it is expanded.
  outerHeight: Specifies the height of the whole browser window in pixels. This outerHeight value includes any/all present toolbar, window horizontal scrollbar (if present) and top and bottom window resizing borders.
  outerWidth: Specifies the width of the whole browser window in pixels. This outerWidth value includes the window vertical scrollbar (if present) and left and right window resizing borders.

it even has a section explicitly documenting the diff between the two:
  https://developer.mozilla.org/en-US/docs/Web/API/Window/open#Note_on_outerHeight_versus_height

window.innerHeight: https://developer.mozilla.org/en-US/docs/Web/API/Window/innerHeight
  Height (in pixels) of the browser window viewport including, if rendered, the horizontal scrollbar.

window.innerWidth: https://developer.mozilla.org/en-US/docs/Web/API/Window/innerWidth
  Width (in pixels) of the browser window viewport including, if rendered, the vertical scrollbar.

window.outerHeight: https://developer.mozilla.org/en-US/docs/Web/API/Window/outerHeight
  Window.outerHeight gets the height in pixels of the whole browser window. It represents the height of the whole browser window including sidebar (if expanded), window chrome and window resizing borders/handles.

window.outerWidth: https://developer.mozilla.org/en-US/docs/Web/API/Window/outerWidth
  Window.outerWidth gets the width of the outside of the browser window. It represents the width of the whole browser window including sidebar (if expanded), window chrome and window resizing borders/handles.

iow, for window.open, MDN says:
  height      == window.innerHeight
  width       == window.innerWidth
  outerHeight == window.outerHeight
  outerWidth  == window.outerWidth
which is what hterm is doing now.  by changing it to do {width,height}={outerWidth,outerHeight}, that's a bug according to MDN.

searching the internet finds some people who note Chrome's misbehavior:
* https://stackoverflow.com/questions/49747453 notes that inner & outer dimensions seem to be the same
*  https://crbug.com/88245#c15  noted that Chrome's handling of height/width are incorrectly used as the outer instead of inner dimensions
*  https://crbug.com/82673  notes that inner & outer dimensions seem to be the same

there is actually a strong preference to specifying the inner dimensions instead of the outer: they're the only dimensions we actually care about because it's the content we're rendering, and  https://crbug.com/88245#c10  notes that the outer dimensions are, by nature of the host OS, never going to be perfect.  but that comment also seems to indicate why this isn't working for us ... extensions/frames w/out chrome/omnibox use a different code path where the calculations are different.

we could deploy a workaround until/if Chrome gets fixed: if the browser is Chrome, pass the outer dimensions to window.open, else pass the inner dimensions.  i'd want to know how Chrome handles this on the open web though vs app/extension to see whether we should further restrict the workaround to only when running as an extension ...
Thank you for the detailed research for this issue. I think you are right: my proposed patch is incorrect. I like the idea to fix it in Chrome instead of workaround in hterm. Is this Chrome bug filed yet?

Sign in to add a comment