Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

innerHeight is incorrect and frozen #203

Closed
Nakilon opened this issue Oct 17, 2021 · 14 comments
Closed

innerHeight is incorrect and frozen #203

Nakilon opened this issue Oct 17, 2021 · 14 comments

Comments

@Nakilon
Copy link
Contributor

Nakilon commented Oct 17, 2021

br = Ferrum::Browser.new   # no matter :headless true or false
br.evaluate "[innerHeight, outerHeight]"

The innerHeight is expected to be smaller than outerHeight: https://developer.mozilla.org/en-US/docs/Web/API/Window/innerHeight
But in fact they are equal and innerHeight is frozen (at 768). When I resize the window it does not change.
Also because of this the third-party JS scripts are unable to correctly obtain the client area and resize the content beyond it.

v 0.11, macOS Big Sur

@Nakilon
Copy link
Contributor Author

Nakilon commented Oct 17, 2021

I've commented this out

ferrum/lib/ferrum/page.rb

Lines 104 to 109 in 7f37640

command("Emulation.setDeviceMetricsOverride", slowmoable: true,
width: width,
height: height,
deviceScaleFactor: 1,
mobile: false,
fitWindow: false)
and the issue is gone. What is that anyway? Its parameters don't even match the https://chromedevtools.github.io/devtools-protocol/tot/Emulation/#method-setDeviceMetricsOverride

Nakilon added a commit to Nakilon/ferrum that referenced this issue Oct 17, 2021
@route
Copy link
Member

route commented Oct 18, 2021

These are params that are passed to Emulation.setDeviceMetricsOverride except slowmoable. Could you write a failing test?

@Nakilon
Copy link
Contributor Author

Nakilon commented Oct 18, 2021

except slowmoable

I don't see fitwiondow neither. I guess it's just outdated. I tried to find what's the point of this command at all but it dates to the first commit of the repo.

Could you write a failing test?

I suppose the simplest assertion is that innerHeight should be smaller than outerHeight in non-fullscreen mode that should be true for any browser.

Other than that probably more reliable would be to craft a HTML page with a node absolutely positioned from 0 to inner... and maybe Ferrum provides a method to see if the node is fully visible. Or maybe if there is only a method to check partial visibility then to craft 4 lines just outside and inside the borders of the rect. Unless these methods depend on the malfunctioning inner...` in the first place

Or maybe try to issue a mouse click event on a positioned element -- pretty sure it won't work if the element is really outside.

Nakilon added a commit to Nakilon/ferrum that referenced this issue Nov 15, 2021
@Nakilon
Copy link
Contributor Author

Nakilon commented Apr 28, 2022

I thought about creating a 1x1 pixel buttons to test if they are clickable but if you zoom it you'd see that buttons are catching the click event on a larger area even with CSS styles applied to make them rectangle and small.

Then I thought that you could screenshot a page and see (with help of chunky_png that is already in the Gemfile) how the rectangle drawn within inner isn't taking all the window space but it appeared that ferrum's screenshot thing is taking exactly the html or body tag size and does not have a clue the real window was larger. (btw, the addition to the initial report: the simplest way to see that it's bugged is that when you resize the window the page remains intact, because those sizes are frozen for the html page runtime). So the only way to screenshot the issue is on the OS level.

P.S.: the "allows the window to be positioned" in bundle exec rspec spec/browser_spec.rb fails on my machine because the initial coordinates appear to be not (0,0) and I'm not sure why the tests even assume that. AFAIK when I was adding/editing those tests I explicitly wrote them in a way that does not assume the starting coordinates, idk why it was changed.

@Nakilon
Copy link
Contributor Author

Nakilon commented Apr 28, 2022

Another thing I see (called binding.irb at the start of some test) is that the things are fixed right after any programmatic resizing:

irb(#<RSpec::ExampleGroups::FerrumBrowser:0x00007ff5c82718f0>):002:0> browser.evaluate "[innerHeight, outerHeight]"
=> [768, 768]
irb(#<RSpec::ExampleGroups::FerrumBrowser:0x00007ff5c82718f0>):003:0> browser.evaluate "[innerWidth, outerWidth]"
=> [1024, 1024]
irb(#<RSpec::ExampleGroups::FerrumBrowser:0x00007ff5c82718f0>):004:0> browser.resize width: 100, height: 100
=> {}
irb(#<RSpec::ExampleGroups::FerrumBrowser:0x00007ff5c82718f0>):005:0> browser.evaluate "[innerWidth, outerWidth]"
=> [100, 514]
irb(#<RSpec::ExampleGroups::FerrumBrowser:0x00007ff5c82718f0>):006:0> browser.evaluate "[innerHeight, outerHeight]"
=> [100, 385]

UPD: nevermind, it's just because of the minimal possible window size. If you increase the window, you'd see the issue is still there:

irb(#<RSpec::ExampleGroups::FerrumBrowser:0x00007ff4b02d6110>):030:0> browser.resize width: 2000, height: 1400
=> {}
irb(#<RSpec::ExampleGroups::FerrumBrowser:0x00007ff4b02d6110>):031:0> browser.evaluate "[innerWidth, outerWidth]"
=> [2000, 2000]
irb(#<RSpec::ExampleGroups::FerrumBrowser:0x00007ff4b02d6110>):032:0> browser.evaluate "[innerHeight, outerHeight]"
=> [1400, 1400]

@Nakilon
Copy link
Contributor Author

Nakilon commented Apr 28, 2022

I would just delete that line if it does not break anything. As I said I see no reason why it's even there, since it's in the first commit. While it breaks the websites layout in the way that page does not fit the window and people have to scroll the page that would not be needed in a normal browser. At least on the headfull macOS. I see no way to fully test it without going to OS level screenshotting, and if not that then:

I suppose the simplest assertion is that innerHeight should be smaller than outerHeight in non-fullscreen mode that should be true for any browser.

@Nakilon
Copy link
Contributor Author

Nakilon commented Mar 29, 2023

OS level screenshotting

For example, craft an html page with with 4 colored pixels on the absolute coordinates, then find the UI element with applescript, screenshot the region and analyze with some chunkypng. Can be done on headfull macOS.

Nakilon added a commit to Nakilon/ferrum that referenced this issue Jun 21, 2023
@route
Copy link
Member

route commented Jul 14, 2023

How about this fix I merged to master #331 was it the reason for your issue?

@route
Copy link
Member

route commented Sep 13, 2023

I believe I had to add it because of Cuprite that don't apply window-size on MacOS. I think after all that time it still doesn't work, so this resize on the browser start was done for that reason.

@route route closed this as completed Sep 14, 2023
@Nakilon
Copy link
Contributor Author

Nakilon commented Sep 16, 2023

How about this fix I merged to master #331 was it the reason for your issue?

I don't believe it's fixed.

my fork:

image

ruby 2.5 + manual patch,
ruby 2.6 + ferrum 0.14,
ruby 2.6 + ferrum master:

image

@route route reopened this Sep 16, 2023
@route
Copy link
Member

route commented Sep 16, 2023

BTW why are you still on ruby 2.6? isn't easy to upgrade?)

@route
Copy link
Member

route commented Sep 16, 2023

Can you attach the script, for me for easy repro?

@Nakilon
Copy link
Contributor Author

Nakilon commented Sep 16, 2023

require "ferrum"
browser = Ferrum::Browser.new window_size: [100, 100], headless: false
browser.go_to "https://vk.com"
gets
bundle install
bundle exec ruby main.rb
source "https://rubygems.org"
gem "ferrum", github: "rubycdp/ferrum"
image
source "https://rubygems.org"
gem "ferrum", github: "nakilon/ferrum"
image

or just any window size and try to resize the window with your mouse

@route
Copy link
Member

route commented Dec 21, 2023

I'm removing this resize call when page is created. I think there was a confusion from my side about --window-size not working on MacOS back then and cuprite requirements.

I think we don't need to support all this in Ferrum as it has to involve less magick than Cuprite. You can resize any page whenever you want and new pages not obliged to inherit this sizу. It's useful for capybara and tests but not for Ferrum. So I'll just move this part to Cuprite.

As for incorrect and frozen innerHeight, well I guess in headless mode it goes nuts, but headful mode is ok. So this issue/question goes to Chrome team.

I hope that removal of the resize call will satisfy you. Let me know please.

route added a commit that referenced this issue Dec 21, 2023
@route route closed this as completed in 447165b Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants