Skip to content

Warn on Protocol-relative URLs#7666

Merged
mikehardy merged 1 commit intoankidroid:masterfrom
david-allison:6102-fix-protocol-relative-urls
Nov 13, 2020
Merged

Warn on Protocol-relative URLs#7666
mikehardy merged 1 commit intoankidroid:masterfrom
david-allison:6102-fix-protocol-relative-urls

Conversation

@david-allison
Copy link
Copy Markdown
Member

@david-allison david-allison commented Nov 13, 2020

Protocol relative URLs didn't work:

Fixes

Fixes #6102

Approach

We add a snackbar to warn the user and explain how to fix this.

In Anki, protocol-relative URLS are converted to https://
In AnkIDroid, they're converted to file://

file:// fails to load the image - we can't redirect from shouldInterceptRequest
Loading synchronously is not viable, so all we can do is use a snackbar.

How Has This Been Tested?

On my phone and API 19 emulator

⚠️ Issue: #7665

image

Learning

Checklist

  • You have not changed whitespace unnecessarily (it makes diffs hard to read)
  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • Your code follows the style of the project (e.g. never omit braces in if statements)
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code
  • UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
  • Strings resources: All strings added as resources are unique or contain a comment for seemingly duplicate strings to explain the difference between both resources

We add a snackbar to warn the user and explain how to fix this.

In Anki, protocol-relative URLS are converted to https://
In AnkIDroid, they're converted to file://

file:// fails to load the image - we can't redirect from shouldInterceptRequest
Loading synchronously is not viable, so all we can do is use a snackbar.

Fixes 6102
Comment thread AnkiDroid/src/main/res/values/constants.xml
Copy link
Copy Markdown
Member

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - what do you think, 2.14-able? This looks like a bug fix to me and is an area well known to you. Might cause a quick follow-on but I can put this out as a beta today

@mikehardy mikehardy merged commit d1c0906 into ankidroid:master Nov 13, 2020
@mikehardy mikehardy added this to the 2.14 release milestone Nov 13, 2020
@david-allison david-allison deleted the 6102-fix-protocol-relative-urls branch November 13, 2020 19:18
@david-allison
Copy link
Copy Markdown
Member Author

@mikehardy It's a bugfix, hopefully non-functional, so 2.14 seems sensible

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.

Reviewer: URLs starting with "//" do not load

3 participants