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

amp="true" doesn't get detected by DeAMP regex #27477

Closed
ShivanKaul opened this issue Dec 20, 2022 · 1 comment · Fixed by brave/brave-core#16431
Closed

amp="true" doesn't get detected by DeAMP regex #27477

ShivanKaul opened this issue Dec 20, 2022 · 1 comment · Fixed by brave/brave-core#16431

Comments

@ShivanKaul
Copy link
Collaborator

ShivanKaul commented Dec 20, 2022

Description

Some AMP pages that should be redirected to non-AMP page aren't. These are AMP pages that have <... amp="true" ...>, they don't get caught by the AMP detection regex.

Steps to reproduce

Make sure DeAMP pref is on in shields settings.

  1. Go to https://www.foxbusiness.com/features/presidential-salaries-from-washington-to-trump.amp
  2. See that it is an AMP page

Actual result

AMP page

Expected result

Not AMP page: URL should redirect to canonical link https://www.foxbusiness.com/features/presidential-salaries-from-washington-to-trump

Issue reproduces how often

Easily

Marking as Android because De-AMP is mainly used on Android.

Test Cases

  1. If De-AMP preference is on, navigate to https://www.foxbusiness.com/features/presidential-salaries-from-washington-to-trump.amp and see that it auto-redirects
  2. Go to google.com and search for "fox business presidential salaries" and click the above link (should be in top few). Check that it also auto-redirects.
@ShivanKaul ShivanKaul added OS/Android Fixes related to Android browser functionality privacy/de-amp labels Dec 20, 2022
@ShivanKaul ShivanKaul self-assigned this Dec 20, 2022
@brave-builds brave-builds added this to the 1.48.x - Nightly milestone Dec 20, 2022
@ShivanKaul ShivanKaul changed the title amp="true" doesn't get detected amp="true" doesn't get detected by DeAMP regex Dec 21, 2022
@stephendonner
Copy link

Verified PASSED using 1.48.134 on a Google Pixel XL running Android 9

Steps:

  1. installed 1.48.134
  2. launched Brave
  3. loaded https://www.foxbusiness.com/features/presidential-salaries-from-washington-to-trump.amp and confirmed it redirected to https://www.foxbusiness.com/features/presidential-salaries-from-washington-to-trump
  4. searched Google.com for fox presidential salaries and tapped the result, which loads the AMP page but redirects also to https://www.foxbusiness.com/features/presidential-salaries-from-washington-to-trump
step 3, DevTools step 3, device step 4, search result step 4, final URL final page
Screenshot 2023-01-22 at 12 46 57 AM Screenshot_20230122-004742 Screenshot_20230122-004912 Screenshot_20230122-010121 Screenshot_20230122-010307

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants