Skip to content
This repository has been archived by the owner on Jan 22, 2022. It is now read-only.

Excessively zoomed-in page(s) #107

Open
jezek opened this issue Nov 20, 2018 · 19 comments
Open

Excessively zoomed-in page(s) #107

jezek opened this issue Nov 20, 2018 · 19 comments
Labels
bug Something isn't working

Comments

@jezek
Copy link
Contributor

jezek commented Nov 20, 2018

Hello, there is at least one site, that is excessively zoomed in and can't be zoomed out.
This is very similiar to issue #12 (or perhaps the same), which is closed as solved.
The old browser in 15.04 (Oxide) got the scaling right. So, I think, it's a regression bug.

Device: FP2
OS: Ubuntu 16.04 (2018-W46)
Site: memrise.com
screenshot20181114_180617523 resized 25 screenshot20181114_180658246 resized 25

Maybe, there are more sites experiencing this zoom bug out there.

This topic is being discussed on this ubports forum.

@halucigenia
Copy link

I have the same issue on https://www.three.co.uk/New_My3/My3_Home
It's a pain as I can't top up from there on my phone.

@casimir7
Copy link

I also have this issue frequently (FP2, OTA-8).
One idea on this:
Most websites choose their representation according to the browser identification code sent.
If you look at [http://sqa.fyicenter.com], it shows:

Device type:
Operating system: Linux
Mozilla/5.0 (Linux; Ubuntu 16.04 like Android 4.4) AppleWebKit/537.36 Chrome/65.0.3325.151 Mobile Safari/537.36)

On an iphone it shows:
Device type: Mobile Phone

It might thus be that some sites do not recognize the device as mobile and display a default Desktop version.
Anyway, it would be good (as a generic solution) to have the zoom not only to work in positive direction but also in negative direction ALWAYS, even at 100%.

@zubozrout
Copy link

zubozrout commented Oct 8, 2019

Hm, I am not sure what's the best way to approach this, but what about this?

/morph-browser/src/app/WebViewImpl.qml
WebViewImpl.qml.txt

It seems to work so maybe someone could have a look, try to think of a better solution / tweak this one and merge it :).

It can be downloaded and tested here: https://www.iubuntu.cz/apps/morph-browser-autozoom.click

On pages like https://pedf.cuni.cz/ which are excessively zoomed in in the current Morph browser this seems to be a working workaround.

@jezek
Copy link
Contributor Author

jezek commented Oct 9, 2019

2019-W41

I've tested this workaround by copying the WebViewImpl.qml.txt to /usr/share/morph-browser/WebViewImpl.qml. It fixes the excessive zoom by setting the zoom factor according to document.body.scrollWidth at the end of onLoadingChanged. So it always overrides domain zoom and default zoom.

I made some changes, this uses autozoom, only if no zoom settings for domain is saved.

WebViewImpl.qml.txt

@zubozrout
Copy link

@jezek Thanks a lot. Yeah, I've noticed that too but you've been faster :) - I haven't had much time for that. Also your code seems more compact and nicer, so thank you for that.

Anyway, I've also noticed (with my code, maybe your changes will fix that) that in some cases if I go to a certain page by clicking an <a> tag link, e.g. through Google search, it doesn't always change the scaling as it should but I have no idea why is that as I haven't seen anything wrong with the code in that perspective.

@zubozrout
Copy link

zubozrout commented Oct 9, 2019

I think we may want this in it as well:

onUrlChanged: {
    /* Reset to default zoom on url change to prevent displaying scaled down pages on load. */
    zoomController.viewSpecificZoom = false;
    zoomController.currentZoomFactor = zoomController.defaultZoomFactor;
    zoomController.refresh();
}

Line 872 here:
WebViewImpl.qml.txt

If you return from a zoomed out page to a certain page working well on mobile, it will not display it zoomed out too but in its correct or zoomed-in form right from the beginning.

Regarding the thing I've mentioned earlier regarding zoom not being updated on page load if you go to the page e.g. through Google or so it seems that onLoadingChanged is not even getting triggered in these cases. I am almost always seeing this on Google results links clicked on after opening the browser. At least until I refresh the respective page, return back to Google and open up the search result page again - then it behaves correctly.

@jezek
Copy link
Contributor Author

jezek commented Oct 10, 2019

@zubozrout
I've made some changes according this Idea, added autozoom to settings and moved the logic a little bit. See xenial_-_autozoom branch on my fork. Can you test it? Thanks.

@zubozrout
Copy link

@jezek Thank you. I've just tested this and it seems to work really well. And I love the idea of having a switch for this in the settings :), great work 👍

@jezek
Copy link
Contributor Author

jezek commented Oct 12, 2019

@zubozrout I've made some tweaks, fixes and utilized your idea using onUrlChanged, to make autozoom more comfortable. Please check it in above mentioned branch and provide some insights, if something feels odd. Thank you.

@zubozrout
Copy link

@jezek Yup, this feels a bit better I suppose :), thank you.

@balcy
Copy link
Collaborator

balcy commented Oct 12, 2019

interesting feature, maybe one small change request:
Since the current domain is always calculated as property, I think it is possible to avoid the additional "UrlUtils.extractHost" by replacing

    onUrlChanged: {
        if (zoomController.autoZoom) {
            var nextDomain = UrlUtils.extractHost(url);
            if (currentDomain !== nextDomain) {
                webview.zoomFactor = zoomController.defaultZoomFactor;
            }
        }
    }

with

   onCurrentDomainChanged: {
           if (zoomController.autoZoom) {
                webview.zoomFactor = zoomController.defaultZoomFactor;
        }
    }

[edit: I have checked that onCurrentDomainChanged gets called just after onUrlChanged]

maybe it is even possible to put the zoom related code from onLoadingChanged to onCurrentDomainChanged, but there might have been a reason to put it there (and load the zoom relatively late, after the load of the page is successful)
But that can be checked at a later point, since it has nothing to with the autozoom feature

@jezek
Copy link
Contributor Author

jezek commented Oct 14, 2019

@balcy Good idea with the onCurrentDomainChanged. Moved the zoom preset load to that function and it did not break the zoom feature. Exact the opposite, site is zoomed before load finished, so no visual quirks are happening.

I also renamed the auto zoom feature, to fit to width and provided a button in the zoom menu. The automatic fit to width feature is now simplified and if enabled, it "launches" fit to width after page load, but only if no specific zoom is stored for domain.

Any feedback?

@zubozrout
Copy link

zubozrout commented Oct 14, 2019

This is very nice :). I didn't now of onCurrentDomainChanged - the documentation doesn't say much actually: https://doc.qt.io/qt-5/qml-qtwebview-webview.html

But this works flawlessly to me, great job :)

@balcy
Copy link
Collaborator

balcy commented Oct 16, 2019

that is a QML feature: all properties with simple data types (e.g. string, bool, int) automatically trigger a changed signal that can be used. (for var it does not work automatically)
so the defined property string currentDomain: UrlUtils.extractHost(webview.url) is updated automatically, as soon as webview.url changes, and then onCurrentDomainChanged can be used to listen to that property change.
https://doc.qt.io/qt-5/qtqml-syntax-signals.html#property-change-signal-handlers

@zubozrout
Copy link

Yup, of course. I know that but was somehow expecting the currentDomain being a native WebView property - haven't noticed it being declared at the top of the file. I've also just noticed it is a readonly property which I thought behaves differently but apparently it has to be writable through the signaling then.

@zubozrout
Copy link

@jezek I think the code is quite nice to be left not-merged. Could you please create a pull-request for this so that maybe someone actually merges this? :)

Unless of course there is some valid reason not to merge it ...

@jezek
Copy link
Contributor Author

jezek commented Nov 4, 2019

@zubozrout I played, with the code, rewrote some former logic, fixed found old bugs and new emerged ones. Today I began test phase and made some final fixes. Only when I'm satisfied with my internal tests, I will write a PR with summary of what was done and why. If everything works right, tomorrow it's finished. ;)

@giiba
Copy link

giiba commented Nov 10, 2019

I don't know if this is related or not but it seems the browser window is reported quite small... I'm using a OPO and know the screen is bigger than this:

screenshot20191110_084157642

Is this by design? I'd prefer the default action for a desktop webpage, to be the page fit to width, and needing to zoom in as opposed to out.

@jezek
Copy link
Contributor Author

jezek commented Nov 11, 2019

@giiba I think the excessive zoom is a bug in qt's web engine (this is maybe related https://bugreports.qt.io/browse/QTBUG-58290). A workaround is currently produced to automatically zoom to fit page width.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

6 participants