Focus-android: Invalid icons of share buttons on Nytimes video section

Created on 14 Jul 2017  路  13Comments  路  Source: mozilla-mobile/focus-android

Devices:
Huawei Honor 8 (Android 6.0)
Samsung Galaxy Note 4 (Android 5.0.1)
build: Focus 1.1. Beta Build #2202

Steps to reproduce:

  1. Open Focus and go to nytimes.com/video.
  2. Scroll down to see the share buttons under each video.

Expected results:
The sharing icons are displayed.

Actual results:
There are invalid icons under the video titles. See screenshot:

screenshot_2017-07-14-16-50-28

bug webview

All 13 comments

I actually saw this as well in www.thestar.com site occasionally. (It shows chinese character instead of a graphical icon). This goes away when I reinstall the app, and it's not clear when the icon gets replaced with asian chars.

What trackers are you blocking in settings? This is often caused by blocking web fonts.

Icons that gets replaces with asian chars are usually social media icons (facebook, twitter, etc.) and I just confirmed that blocking web fonts causes it on thestar.com. (perhaps a warning would be nice?)
EDIT: oh wait, it already have the warning.

Yeah. There's also #834 for improving the explanations.

Turned off all trackers in Settings and this still occurs.

hmm, sv-orhorvath is right, this is different from thestar.com issue. chinese chars show when web font blocking is disabled.

Oh, right. And it doesn't happen on Chrome.

I think the problem is we're blocking cookies: I can reproduce on Chrome with cookies disabled.

on iOS, it just doesn't display anything where icons should be- we should file a mirror bug in iOS as well

I fixed this by enabling dom storage in settings in WebViewProvider. Is this something that we would want to do in general? Not sure what else dom storage is used for, but I assume we clear it with WebStorage.getInstance().deleteAllData() ?

Here's some more information I found about dom storage API. I am not sure if there are any security concerns or extra steps in addition to WebStorage.getInstance().deleteAllData() we would have to take to clear storage after the session.

https://developer.mozilla.org/en-US/docs/Web/API/Web_Storage_API
http://viralpatel.net/blogs/introduction-html5-domstorage-api-example/

@ekager I wrote an automated test to make sure we are clearing the session store after every session (Set you as a reviewer). This test passes with your patch: We store and keep the value and remove it after the session has been erased. Can you create a PR for your patch? :)

Merged!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

mcomella picture mcomella  路  5Comments

Benestar picture Benestar  路  7Comments

zekooooo picture zekooooo  路  7Comments

abusedcharacter picture abusedcharacter  路  5Comments

uncertainquark picture uncertainquark  路  6Comments