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

Request Mobile Site preference #42

Merged
merged 7 commits into from
Dec 31, 2017
Merged

Conversation

jondashkyle
Copy link
Contributor

@jondashkyle jondashkyle commented Dec 7, 2017

That was pretty easy. This creates the ability to set the useragent (UA) to load mobile sites. It does so by updating the webview’s useragent. For a good example navigate to Twitter, The New York Times, Google or Github.

A few things to note:

Preferences

Updated the preferences with a new one called request_mobile_site, which defaults to false. There is also a menu item toggle under Tools called Request Mobile Site. The shortcut is CmdOrCtrl+Alt+U, as M was taken. U for useragent? Not the clearest… Toggling this option updates the UA and reloads the site to reflect the change.

UA Storage

We must store the mobile/desktop useragents. I couldn’t quickly find a way of requesting the default Electron UA. The webview must be mounted to the DOM before you can request it’s current UA. To get around this, I define the UA through setAttribute, as suggested here to avoid first load without the correct UA defined, effectively overwriting any requests for the current UA as a way of storing the default/desktop.

Default (desktop) is what my current machine reads:

 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_5) AppleWebKit/537.36 (KHTML, like Gecko) Oryoki/0.2.1 Chrome/58.0.3029.110 Electron/1.7.9 Safari/537.36'

Mobile is an iPhone running iOS 11 in Safari:

'Mozilla/5.0 (iPhone; CPU iPhone OS 11_0 like Mac OS X) AppleWebKit/604.1.38 (KHTML, like Gecko) Version/11.0 Mobile/15A356 Safari/604.1'

This is probably fine for OSX releases, but we’ll want to find a better way of doing this if building for another OS.

@jondashkyle jondashkyle mentioned this pull request Dec 7, 2017
@thmsbfft
Copy link
Owner

Hey this is great! Thanks a bunch. Sorry it took so long for me to check it out. I've tested it on your branch and it seems to work, but for some reason I'm having issues like these:

– Preference can carry to new windows
– Preference doesn't work on first window
– Doesn't switch back to desktop site from website.com/mobile
– Navigating to a different website still loads desktop

So, generally, it does what you'd think after a couple tries, but the behavior isn't consistent. Not sure how to approach this—did you run into any of these issues before?

@thmsbfft
Copy link
Owner

Here's an alternative scenario:
– Use Request Mobile Site as an action rather than a toggle
– Reset the UA often or only set it for one window and one website

That way you can just smash the shortcut to get a mobile redirect on a site you just loaded—maybe more forgiving?

@jondashkyle
Copy link
Contributor Author

Hey! Thanks for giving it a look. I could see this being an array of sites in your user preferences json. Triggering the action adds or removes the site from that array. Having used this over the past two weeks I really do find it useful; having it reset or only applied per-window would be an annoyance.

  • I’m also seeing the bug where the preference does not get applied to the first window.
  • Toggling off works depending upon the site. For instance, the nytimes redirects you to mobile.nytimes.com, as you point out. Unsure how we could elegantly resolve that.
  • Can you clarify “Navigating to a different website still loads desktop”? This is true for me if continuing to navigate in the original window, however opening a new window resolves this. Likely related to the first bug on this list.

To summarize, the primary bug of requiring a new window to take effect can likely be resolved within toggleRequestMobile() under /lib/components/view.js. The functionality to toggle per-site and store in user preferences sounds useful!

@thmsbfft
Copy link
Owner

Gotcha—my issue with “Navigating to a different website still loads desktop” was related to the original window bug, I think. Just got weirded out by the behavior of the thing.

  • Agree that the main issue is the UA spoofing not working on the app's first window.
    Replacing load(webview.getURL()) with reload() fixes this for me (don't ask me why). With that fix, we could make "Request Mobile Website" a global toggle (for all windows), and we'd be set, no?

  • On toggling off: looked into it a bit and it seems browsers like Chrome or Safari do more than switching UA + Refresh, though. They seem to remember what was your original request and can revert to that when disabling the browsing mode, except if you explicitly typed mobile.website.com. Not sure we need all that, though.

@jondashkyle
Copy link
Contributor Author

  • Glad to hear reload() fixes it! Was hoping it’d be a simple (although mysterious) one.
  • Oh interesting. I think that would be a good feature to add progressively.

I think that this is a nice first implimentation. Perhaps if there is a roadmap for additional functionality we could append that second note!

…pp-wide parameters without adding weight to the code.
…g it will reload all windows, and new windows will take the parameter from the app's menu. the preference still works as expected. 2) the toggle is disabled on windows first load, to avoid an error when setting the UA before the webview session is ready.
@thmsbfft
Copy link
Owner

thmsbfft commented Dec 30, 2017

About those commits earlier: I couldn't find a way to enforce the UA on a window basis (once it's set on one webview, all other ones follow), so I made the changes to make it an app-wide parameter.

You can test it the following way:
– Open the app (with preference set to false)
– Navigate to a wikipedia page or such
– Open a new window, navigate to twitter or such
– Toggle Request Mobile Site on
– All windows will reload with the mobile version of the pages
– New windows will also load mobile versions

Disabling Request Mobile Site roughly works as expected (disables UA on future requests and reloads current urls).

The key in oryoki-preferences.json allow to start the app with the toggle on.

@jondashkyle Will let you have a look at it, and if that looks okay I'll merge into /dev.

@jondashkyle
Copy link
Contributor Author

This looks great! Clicked around a bit and was able to replicate everything you mentioned above. Seems good to me!

@thmsbfft thmsbfft merged commit dcc5e2b into thmsbfft:dev Dec 31, 2017
@sonn-gamm
Copy link

Is this already available in the 0.2.1 build?

I added "request_mobile_site" : true (and tried with false as well) in oryoki's json prefs but can't find the menu item under Tools. Also after quitting and reopening the app.

@thmsbfft
Copy link
Owner

thmsbfft commented Jan 4, 2018

Afraid you'd have to build from source on /dev for this. I'd like to fix a few things before releasing 0.2.2, hopefully soon.

@sonn-gamm
Copy link

Thanks for replying, no problem! Could you point me out how to do it? Never built up an electron app from source.

@thmsbfft
Copy link
Owner

thmsbfft commented Jan 4, 2018

I've added some instructions there: https://github.com/thmsbfft/oryoki/blob/dev/README.md#build-from-source

If you follow that from the dev branch, you should be able to build. Not fully tested on that branch obviously, so things might break a bit. Hope this helps!

@sonn-gamm
Copy link

Working well! I'd suggest to add to the instructions for building the app from source, to add

  • git branch -a
  • git checkout dev

after you did git clone.

Thanks!

@thmsbfft
Copy link
Owner

thmsbfft commented Jan 5, 2018

Added that, thanks for the suggestion—and glad packaging worked out!

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

Successfully merging this pull request may close these issues.

3 participants