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.

Sign up for GitHub

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

Jump to bottom

[De-AMP] Follow up issues for Android #21643

Closed
ShivanKaul opened this issue Mar 14, 2022 · 4 comments · Fixed by brave/brave-core#13001
Closed

[De-AMP] Follow up issues for Android #21643

ShivanKaul opened this issue Mar 14, 2022 · 4 comments · Fixed by brave/brave-core#13001
Assignees
@ShivanKaul
Labels
OS/Android Fixes related to Android browser functionality privacy/de-amp QA Pass - Android ARM QA/Yes release-notes/include
Milestone
1.38.x - Release

Comments

@ShivanKaul
Copy link
Collaborator

Noting down follow ups for brave/brave-core#11750

  • Add settings UI for Android
  • Manual test on Android
  • Enable feature flag on Android
@ShivanKaul ShivanKaul added OS/Android Fixes related to Android browser functionality OS/Desktop labels Mar 14, 2022
@ShivanKaul ShivanKaul self-assigned this Mar 14, 2022
@ShivanKaul ShivanKaul mentioned this issue Mar 16, 2022
Closed
@anthonypkeane anthonypkeane mentioned this issue Apr 12, 2022
Closed
@anthonypkeane
Copy link

for setting text see: #20458 (comment)

@ShivanKaul ShivanKaul mentioned this issue Apr 12, 2022
Merged
25 tasks
@ShivanKaul ShivanKaul added this to the 1.39.x - Nightly milestone Apr 15, 2022
@brave-builds brave-builds mentioned this issue Apr 19, 2022
Merged
7 tasks
@kjozwiak
Copy link
Member

@srirambv @Uni-verse in case you guys run through the above on 1.38,x, cases can be found via brave/brave-core#13001 (comment) which were copied over from brave/brave-core#11750 (comment).

Above requires 1.38.105 or higher for 1.38.x verification 👍

@Uni-verse
Copy link
Contributor

Uni-verse commented Apr 25, 2022 •
edited
Loading

Verification PASSED using 1.38.106, Chromium 101.0.4951.41 on Samsung GS 21 running Android 12

Duplicated the STR/Cases by @kjozwiak which were outlined via brave/brave-core#13001 (comment) in the testing on Nightly.

Test Case 1: Inspecting AMP elements on pages
  1. Disable Auto-redirect AMP pages in brave://settings/privacy
  2. Disable Enable De-AMP flag - brave://flags/#brave-de-amp
  3. Relaunch Brave
  4. Visit AMP page https://shivankaul.com/brave/de-amp/test-small-amp.html.
    • Inspected element and amp attribute will be shown in the top level <html amp> tag for the AMP page and has canonical link that redirects to html
  5. Close the tab and visit another AMP page https://shivankaul.com/brave/de-amp/test-small-amp-with-lightning.html
    • Inspected element and it contains lightning symbol in the top-level <html ⚡> tag for the AMP page and has canonical link that redirects to html
  6. Close the tab and visit https://shivankaul.com/brave/de-amp/test-no-canonical-link.html
    • Inspected element and it doesn't contain any canonical links or top-level tags
  7. Close the tab and visit https://shivankaul.com/brave/de-amp/test-wrong-scheme.html
    • Inspected element for canonical link that is non-HTTP(s) - <link rel="canonical" href="ipfs://QmcniBv7UQ4gGPQQW2BwbD4ZZHzN3o3tPuNLZCbBchd1zh">
  8. Close the tab and visit https://shivankaul.com/brave/de-amp/test-small-amp-same-canonical.html
    • Inspected the element and it has canonical link that is pointed to original/same URL
  9. Close the tab and visit https://www.buzzfeed.com/amphtml/ryanschocket2/snl-spoofs-will-smith-chris-rock-slap
    • Inspected the element and it has canonical link that is pointed https://www.buzzfeed.com/ryanschocket2/snl-spoofs-will-smith-chris-rock-slap
example example example example example example example example
Screen Shot 2022-04-25 at 1 02 27 PM Screen Shot 2022-04-25 at 12 33 50 PM Screen Shot 2022-04-25 at 12 37 13 PM Screen Shot 2022-04-25 at 12 38 47 PM Screen Shot 2022-04-25 at 12 41 19 PM Screen Shot 2022-04-25 at 12 43 05 PM Screen Shot 2022-04-25 at 12 45 25 PM Screen Shot 2022-04-25 at 12 48 23 PM
Test Case 2: Test Case 2: Verifying AMP page will not be redirected when both de-AMP flag and auto-redirect AMP page are disabled
  1. Disable Auto-redirect AMP pages in brave://settings/privacy
  2. Disable #brave-de-amp flag in brave://flags/
  3. Relaunch the browser
  4. Visit https://shivankaul.com/brave/de-amp/test-small-amp.html
  5. Inspect the page and it showedamp as attribute
  6. Contains canonical in the tag
  7. AMP Page is not redirected to brave.com

Screen Shot 2022-04-25 at 5 02 34 PM

Test Case 3: Verifying de-AMPing works as expected i.e redirects to brave.com with default settings

Prerequisite: De-AMP enabled in setting, flag enabled

  1. Visit https://shivankaul.com/brave/de-amp/test-small-amp.html
  2. De-AMPing worked i.e AMP page is redirected to brave.com, as it contained the markup for top-level
  3. The markup was verified in Test Case 1 for the above AMP page and amp attribute will be shown in the top level tag for the AMP page and has canonical link that redirects to html

Screen Shot 2022-04-25 at 5 14 43 PM

Test Case 4: Verifying de-AMPing works if the AMP page contains the lightning emoji instead of "amp" as attribute

Prerequisite: De-AMP enabled in setting, flag enabled

  1. Visit https://shivankaul.com/brave/de-amp/test-small-amp-with-lightning.html
  2. De-AMPing worked i.e AMP page is redirected to brave.com as it contained markup for top-level <html ⚡> tag
  3. The markup was verified in Testcase 1 for the above AMP page has lightning <html ⚡> tag and contains canonical in the tag for redirect
1 2
Screen Shot 2022-04-25 at 12 38 47 PM Screen Shot 2022-04-25 at 5 14 43 PM
Test Case 5: Verifying de-AMPing will not work for AMP page with no canonical link

Prerequisite: De-AMP enabled in setting, flag enabled

  1. Visit https://shivankaul.com/brave/de-amp/test-no-canonical-link.html
  2. De-AMPing did not work i.e AMP page is not redirected since there is no canonical link tag
  3. The markup was verified in Test Case 1, for the above AMP page has no canonical link tag hence no redirect available

Screen Shot 2022-04-25 at 12 41 19 PM

Test Case 6: Verifying de-AMPing will not work for AMP page that includes canonical link with scheme != HTTP(S)

Prerequisite: De-AMP enabled in setting, flag enabled

  1. Visit https://shivankaul.com/brave/de-amp/test-wrong-scheme.html
  2. De-AMPing did not work i.e AMP page is not redirected since the AMP page has canonical link with scheme non HTTP(S)
  3. Verified in Test Case 1, Step 8, that the above AMP page has canonical link with scheme non HTTP(S)

Screen Shot 2022-04-25 at 12 43 05 PM

Test Case 7: Verifying de-AMPing will not work for AMP page with canonical link same as original URL

Prerequisite: De-AMP enabled in setting, flag enabled

  1. Visit https://shivankaul.com/brave/de-amp/test-small-amp-same-canonical.html
  2. "De-AMPing did not work i.e AMP page is not redirected since the AMP page has canonical link same as original URL
  3. Verified in Test Case 1, Step 9, that the above AMP page has canonical link same as original URL

Screen Shot 2022-04-25 at 12 45 25 PM

Test Case 8: Verifying that navigating backward and forward ignores the AMP page in history

Prerequisite: De-AMP enabled in setting, flag enabled

  1. Visited example.com
  2. Visited AMP page in the same tab- https://shivankaul.com/brave/de-amp/test-small-amp.html
  3. Page is redirected to brave.com
  4. Visited https://basicattentiontoken.org in the same tab
  5. clicked back in the browser
  6. Returned to brave.com
  7. Clicked back again
  8. AMP page https://shivankaul.com/brave/de-amp/test-small-amp.html is ignored and is not listed in history
  9. Clicked forward
  10. User is taken brave.com
  11. Clicked forward again
  12. Returned to https://basicattentiontoken.org
  13. AMP page https://shivankaul.com/brave/de-amp/test-small-amp.html is ignored and is not listed in history
  14. Verified the history lists example.com, brave.com, and basicattentiontoken.org

Screen Shot 2022-04-25 at 5 54 39 PM

Test Case 9: Verifying restore de-AMPed page
  1. Visit https://shivankaul.com/brave/de-amp/test-small-amp.html
  2. Page should be redirected to brave.com
  3. Opened a few tabs
  4. Close brave.com tab
  5. Restore the tab
  6. De-AMPed page loaded

Screen Shot 2022-04-25 at 5 59 09 PM

Test Case 10: Verifying de-AMPing works well with small pages < 64 K works expected
  1. Visit https://shivankaul.com/brave/de-amp/test-small-amp.html
  2. De-AMPing worked i.e AMP page is redirected to brave.com

Screen Shot 2022-04-25 at 6 01 24 PM

Test Case 11: Verifying large AMP page (> 64K) works as expected
  1. Visit https://www.buzzfeed.com/amphtml/ryanschocket2/snl-spoofs-will-smith-chris-rock-slap
  2. De-AMP ing worked i.e AMP page is redirected to https://www.buzzfeed.com/ryanschocket2/snl-spoofs-will-smith-chris-rock-slap
  3. The markup was verified in Test Case 1 for the above AMP page has canonical link to https://www.buzzfeed.com/ryanschocket2/snl-spoofs-will-smith-chris-rock-slap

Screen Shot 2022-04-25 at 6 06 34 PM

Test Case 12: Verifying de-AMPing will not work with Auto-redirect AMP pages ON and de-AMPing feature flag ON (default)
  1. Disable Auto-redirect AMP pages inbrave://settings/privacy
  2. Keep Enable De-AMP flag as a default in brave://flags/#brave-de-amp
  3. Visit AMP page https://shivankaul.com/brave/de-amp/test-small-amp.html
  4. Page is not redirected to brave.com

Screen Shot 2022-04-25 at 6 10 19 PM

Test Case 13: Verifying de-AMPing will not work with Auto-redirect AMP pages ON and de-AMPing feature flag OFF
  1. Enable Auto-redirect AMP pages inbrave://settings/privacy
  2. Disable Enable De-AMP flag in brave://flags/#brave-de-amp
  3. Visit AMP page https://shivankaul.com/brave/de-amp/test-small-amp.html
  4. Page is not redirected to brave.com

Screen Shot 2022-04-25 at 6 15 27 PM

@kjozwiak
Copy link
Member

After speaking with @ShivanKaul, running through the mobile cases should be enough and tablet can be skipped. Tablet doesn't use AMP as much as the mobiles due to the view usually being Desktop due to the bigger screens. However, double checked and ensured that the feature has been enabled by default.

@kjozwiak kjozwiak mentioned this issue Apr 27, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ShivanKaul ShivanKaul

Labels
OS/Android Fixes related to Android browser functionality privacy/de-amp QA Pass - Android ARM QA/Yes release-notes/include
Projects
None yet
Milestone
1.38.x - Release
Development

Successfully merging a pull request may close this issue.

Add De-AMP setting for Android, enable by default brave/brave-core
4 participants
@kjozwiak @ShivanKaul @anthonypkeane @Uni-verse