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

LinkHints gets a wrong view area if page is zoomed on Chrome for macOS #119

Closed
agzam opened this issue Feb 2, 2020 · 45 comments
Closed

Comments

@agzam
Copy link

agzam commented Feb 2, 2020

It's missing urls sometimes.
Here's for example of a GitHub page, when f pressed, you can clearly see that all wiki links are ignored.

Screen Shot 2020-02-02 at 2 27 54 PM

Just in case I checked if the same thing is also broken in Vimium and it is not. Vimium still recognized all links on the page

upd: Apparently when content zoomed-in to the point when devicePixelRatio exceeds 2, link hinting breaks.

upd 2: This turned out to be specific to OS X and the way how it handles scaling. I failed to reproduce the issue on any other display, it only appears on the Macbook's internal retina display with scaled resolution.

@agzam

This comment has been minimized.

@Monirzadeh
Copy link

Monirzadeh commented Feb 2, 2020

just for test
press F2 in hint mode makes any different?
Update: can you disable all other extension and test again?

@agzam

This comment has been minimized.

@agzam

This comment has been minimized.

@Monirzadeh
Copy link

Monirzadeh commented Feb 2, 2020

it's the same - incognito in Chrome, private in Brave, still missing the links. F2 has no effect whatsoever, what that suppose to do?

i just test , may be it is like this situation here

@agzam

This comment has been minimized.

@Monirzadeh
Copy link

Monirzadeh commented Feb 2, 2020

i test same page in brave (Vimium wiki) and it's work for me without any problem so i think something interrupt the extension
this is my screenshot on brave and Vimium C 1.80.1 on Linux
Screanshot-2020-02-03-02-44-24-389452987

@gdh1995
Copy link
Owner

gdh1995 commented Feb 2, 2020 via email

@agzam

This comment has been minimized.

@gdh1995
Copy link
Owner

gdh1995 commented Feb 3, 2020

Hello, I didn't see the snapshot when I replied through email. According to these snapshot, it should be because Vimium C gets a wrong "visual viewport" (visible area).

So what's your screen DPI, browser and its version code, opearting system and its version ? Thanks for this report.

@agzam

This comment has been minimized.

@gdh1995
Copy link
Owner

gdh1995 commented Feb 3, 2020

What's the result of visualViewport and devicePixelRatio?

@agzam

This comment has been minimized.

@agzam

This comment has been minimized.

@agzam

This comment has been minimized.

@gdh1995
Copy link
Owner

gdh1995 commented Feb 3, 2020

Is the size of your Chrome window maximized?

On my computer, visualViewport.width is near to innerWidth (their delta means a scrollbar), and innerWidth * devicePixelRatio is approximate to the width of screen resolution.

So could you provide info of screen, innerWidth, innerHeight, visualViewport of a tab in a maximized Chrome window (the Developer Tools should in a seperate window)?

@agzam

This comment has been minimized.

@agzam

This comment has been minimized.

@agzam

This comment has been minimized.

@agzam

This comment has been minimized.

@gdh1995
Copy link
Owner

gdh1995 commented Feb 3, 2020

Have you zoomed-out on github.com by Command+-?

@agzam

This comment has been minimized.

@agzam

This comment has been minimized.

@agzam agzam changed the title Missing links Hinting fails with zoom levels above certain threshold Feb 3, 2020
@gdh1995
Copy link
Owner

gdh1995 commented Feb 3, 2020

vimium_c-1.80.1-dist-no-visualviewport.zip

What about this version? I removed visualViewport from LinkHints.

@agzam

This comment has been minimized.

@agzam

This comment has been minimized.

@gdh1995
Copy link
Owner

gdh1995 commented Feb 3, 2020

What it will be if zoom-out and make devicePixelRatio be 1 on a page's web console, on your Mac's bulitin monitor?

@agzam

This comment has been minimized.

@agzam
Copy link
Author

agzam commented Feb 3, 2020

I'm even trying to zoom on my external monitor so its devicePixelRatio goes above 3 and it still hints all visible links.

@gdh1995
Copy link
Owner

gdh1995 commented Feb 3, 2020

Have you tested a case when your external monitor was disconnected?

@agzam
Copy link
Author

agzam commented Feb 3, 2020

Yes, I can reproduce the bug on Macbook's display with good consistency (with or without external display connected). Alas, I'm failing to repro it on any other display. I will try this tomorrow at work (I have some shitty old monitors there)

@agzam

This comment has been minimized.

@gdh1995
Copy link
Owner

gdh1995 commented Feb 3, 2020

[+getComputedStyle(document.documentElement).zoom, devicePixelRatio] when zoom in/out?

@agzam

This comment has been minimized.

@agzam
Copy link
Author

agzam commented Feb 3, 2020

Holy shit. This is what I did:

image

And it now works fine. It's a scaling problem with OSX and Chromium I guess.
I am very sorry for causing you so much trouble with this. It's probably better if fixed, but I guess it's not such a big deal. I'm not sure though how exactly Vimium (classic) handles this.

@gdh1995
Copy link
Owner

gdh1995 commented Feb 3, 2020

Oops, then this issue is because of one of my tricks to support the zoom CSS style on document.documentElement:

  • on my Win 10, there's a period that document.documentElement's zoom keeps the same as devicePixelRatio, if only there're no CSS styles setting it.
  • but this "feature" (or Chrome bug?) is not reproduced on Chrome 79, my Win 10...
  • then Vimium C tends to think "there's a zoom from page styles" since getComputedStyle(document.documentElement).zoom returns a different value, so it crops some right and bottom hints.

I'll fix it in days. Thanks for your patience and tests.

BTW, this feature is since Chrome 61 on Windows:

// a bug that special style.zoom may not work is fixed since MinASameZoomOfDocElAsdevPixRatioWorksAgain
MinDevicePixelRatioImplyZoomOfDocEl = 61,

@agzam
Copy link
Author

agzam commented Feb 3, 2020

Ah, okay. Yeah, feel free to ping me anytime if you want me to run another test. It would suck if OSX users with Chromium browsers decide not to use the extension because of this bug. Thank you for your help.

@gdh1995
Copy link
Owner

gdh1995 commented Feb 3, 2020

Um, I want to know what's [+getComputedStyle(document.documentElement).zoom, devicePixelRatio] when screens' system DPIs are larger than 1, and the test page is not zoomd (aka. zoom level = 100%) by Chrome:

  • on the built-in monitor, and not "open in low resolution"
  • on the external monitor, and not "open in low resolution"
  • on the built-in monitor, and enable "open in low resolution"
  • on the external monitor, and enable "open in low resolution".

@gdh1995
Copy link
Owner

gdh1995 commented Feb 3, 2020

As far as I've got, the wrong result occurs when:

  1. enable the "open in low resolution" checkbox
  2. open Chrome on the bulit-in monitor
    1. then getComputedStyle(document.documentElement).zoom will return the browser-level zoom ratio, while is not equal with JavaScript's window.devicePixelRatio (which should be physical DPI * browser-level zoom).
  1. if zoom-in a page and make its browser-level zoom ratio larger than 1 (like 110%, 125%, 150%...),
    1. then getComputedStyle(document.documentElement).zoom will return 1.1 / 1.25 / 1.5 / ...

Are these right? If they're, then there seems no good way to detect such a bug, and maybe a new option is necessary to manually work it around (let you tell Vimium C what's the basic DPI).

@agzam
Copy link
Author

agzam commented Feb 4, 2020

I'm not following you, with "open in low resolution" I can no longer reproduce the bug. Only when the checkbox is "off" I can reproduce it, and only on the Macbooks internal display. Maybe it's really not worth wasting time trying to fix it?

@gdh1995
Copy link
Owner

gdh1995 commented Feb 5, 2020 via email

gdh1995 added a commit that referenced this issue Feb 5, 2020
now accept a zoom only if it's from the DOM attribute of "style".

See
#119 (comment) .
@gdh1995
Copy link
Owner

gdh1995 commented Feb 5, 2020

So please try this - I've marked most getComputedStyle(documentElement).zoom untrusted - then it should work when you uncheck the "low resolution" box.

vimium_c-1.80.1-chrome-6ec887c.zip

@gdh1995 gdh1995 changed the title Hinting fails with zoom levels above certain threshold LinkHints gets a wrong view area if page is zoomed on Chrome for macOS Feb 9, 2020
gdh1995 added a commit that referenced this issue Feb 9, 2020
there're at least 3 kinds of docEl's zoom, even when it's not set:
* `pageZoom * screen DPI`, on Chrome for Windows
* `pageZoom` only, while renderer follows screen DPI, on Chrome for macOS
* `pageZoom` only, and render does not follow screen DPI
  * if `--enable-use-zoom-for-dsf=false` or "Open in Low Resolution" on macOS
@agzam
Copy link
Author

agzam commented Feb 12, 2020

Tried that. "Open in low resolution" is unchecked. You have fixed it! Thank you very much!

chromium-c

@gdh1995
Copy link
Owner

gdh1995 commented Feb 12, 2020

OK.

The v1.80.2 is being published to Chrome Web Store (waiting for verifying). However it can not support zoom changes - only the first page-zoom value will be recognized, because of a typo. This will be fixed in a next version of Vimium C.

@gdh1995
Copy link
Owner

gdh1995 commented Feb 24, 2020

v1.80.3 has been published, and it supports: 1. show Link Hints and hide; 2. change page zoom level; 3. show LinkHint again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants