Skip to content

Allow capturing of multiple capture groups for debounce action regex-path #24750

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
Closed
ShivanKaul opened this issue Aug 17, 2022 · 4 comments · Fixed by brave/brave-core#14687
Closed

Allow capturing of multiple capture groups for debounce action regex-path #24750

ShivanKaul opened this issue Aug 17, 2022 · 4 comments · Fixed by brave/brave-core#14687
Assignees
@ShivanKaul
Labels
OS/Android Fixes related to Android browser functionality OS/Desktop privacy/debounce URL debouncer QA Pass - Android ARM QA Pass - Android Tab QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-All-Platforms QA/Yes release-notes/exclude
Milestone
1.44.x - Release

Comments

@ShivanKaul
Copy link
Collaborator

ShivanKaul commented Aug 17, 2022 •
edited
Loading

To debounce URLs like https://topnews--ru-ru.turbopages.org/topnews-ru.ru/s/2022/08/15/djoan-royling-prigrozili-smertu-za-podderjky-ryshdi-ty-sledyushaia/, we need to remove the /s/. We can do this by allowing for multiple capture groups for the action regex-path and then concatenating the matches.

Currently the action validates that there is only one capture group.

Test plan in PR description: brave/brave-core#14687
@ShivanKaul ShivanKaul added OS/Android Fixes related to Android browser functionality OS/Desktop privacy/debounce URL debouncer labels Aug 17, 2022
@ShivanKaul ShivanKaul self-assigned this Aug 17, 2022
@ShivanKaul ShivanKaul mentioned this issue Aug 17, 2022
Merged
25 tasks
@brave-builds brave-builds added this to the 1.44.x - Nightly milestone Aug 19, 2022
@MadhaviSeelam
Copy link

MadhaviSeelam commented Sep 2, 2022 •
edited
Loading

Verification PASSED using

Brave | 1.44.73 Chromium: 105.0.5195.68 (Official Build) beta (64-bit)
-- | --
Revision | ad13e82529051bac6a0e65f455e6d7a1e5fd7938-refs/branch-heads/5195@{#903}
OS | Windows 11 Version 21H2 (Build 22000.856)

Filed: #25259

Verified with the testplan brave/brave-core#14687 (comment)

  • Install 1.44.73
  • launch Brave

URL 1 - PASSED

initial url redirect url
image image

URL 2 - PASSED

initial url redirect url
image image

@MadhaviSeelam MadhaviSeelam added the QA/In-Progress Indicates that QA is currently in progress for that particular issue label Sep 2, 2022
@stephendonner
Copy link

Verified PASSED using

Brave 1.44.79 Chromium: 105.0.5195.102 (Official Build) beta (x86_64)
Revision 4c16f5ffcc2da70ee2600d5db77bed423ac03a5a-refs/branch-heads/5195_55@{#4}
OS macOS Version 13.0 (Build 22A5331f)

Steps:

  1. installed 1.44.79
  2. launched Brave
  3. loaded https://topnews--ru-ru.turbopages.org/topnews-ru.ru/s/2022/08/15/djoan-royling-prigrozili-smertu-za-podderjky-ryshdi-ty-sledyushaia/
  4. confirmed it redirected to https://topnews-ru.ru/2022/08/15/djoan-royling-prigrozili-smertu-za-podderjky-ryshdi-ty-sledyushaia/
  5. loaded https://08-mchs-gov-ru.turbopages.org/08.mchs.gov.ru/s/deyatelnost/press-centr/novosti/4726459
  6. confirmed it redirected to https://08.mchs.gov.ru/deyatelnost/press-centr/novosti/4726459
step 3 step 4 step 5 step 6
Screenshot 2022-09-08 at 1 40 38 PM Screenshot 2022-09-08 at 1 40 45 PM Screenshot 2022-09-08 at 1 41 36 PM Screenshot 2022-09-08 at 1 41 43 PM

@stephendonner stephendonner added QA Pass-macOS and removed QA/In-Progress Indicates that QA is currently in progress for that particular issue labels Sep 8, 2022
@MadhaviSeelam
Copy link

MadhaviSeelam commented Sep 16, 2022 •
edited
Loading

Verification PASSED using

Brave | 1.44.86 Chromium: 105.0.5195.127 (Official Build) beta (64-bit)
-- | --
Revision | 912488396852bf658ab32465980c0b93a3c27a83-refs/branch-heads/5195@{#1109}
OS | Linux
  • Install 1.44.86
  • launch Brave

URL 1 - PASSED

initial url redirect url
url url

URL 2 - PASSED

initial url redirect url
url url

@kjozwiak
Copy link
Member

kjozwiak commented Sep 22, 2022 •
edited
Loading

Verification PASSED on Pixel 6 running Android 13 using the following build(s):

Brave | 1.44.95 Chromium: 106.0.5249.40 (Official Build) (32-bit)
--- | ---
Revision | fab1d91915d2722d6339aaa7f4e9ce44f1e9b103-refs/branch-heads/5249@{#442}
OS | Android 13; Build/T1B1.220819.006

Went through the STR/Cases outlined via brave/brave-core#14687 (comment) and ensured the following:

Test Case #1 (URL #1)

  • launch 1.44.95 Chromium: 106.0.5249.40 and restarted Brave
  • ensured that BraveDebounceStudy:Enabled via brave://version

Ensured that visiting https://topnews--ru-ru.turbopages.org/topnews-ru.ru/s/2022/08/15/djoan-royling-prigrozili-smertu-za-podderjky-ryshdi-ty-sledyushaia redirected to https://topnews-ru.ru/2022/08/15/djoan-royling-prigrozili-smertu-za-podderjky-ryshdi-ty-sledyushaia without any issues.

Test Case #2 (URL #2)

  • launch 1.44.95 Chromium: 106.0.5249.40 and restarted Brave
  • ensured that BraveDebounceStudy:Enabled via brave://version

Ensured that visiting https://08-mchs-gov-ru.turbopages.org/08.mchs.gov.ru/s/deyatelnost/press-centr/novosti/4726459 redirected to https://08.mchs.gov.ru/deyatelnost/press-centr/novosti/4726459 without any issues.

Test Case #3 - Regex (multi)

Example (Disabled) Example (Enabled)
Screenshot_20220921-204246 Screenshot_20220921-203712

Verification PASSED on Samsung Tab S8 Ultra running Android 12 using the following build(s):

Brave | 1.44.95 Chromium: 106.0.5249.40 (Official Build) (32-bit)
--- | ---
Revision | fab1d91915d2722d6339aaa7f4e9ce44f1e9b103-refs/branch-heads/5249@{#442}
OS | Android 12; Build/SP2A.220305.013

Went through the STR/Cases outlined via brave/brave-core#14687 (comment) and ensured the following:

Test Case #1 (URL #1)

  • launch 1.44.95 Chromium: 106.0.5249.40 and restarted Brave
  • ensured that BraveDebounceStudy:Enabled via brave://version

Ensured that visiting https://topnews--ru-ru.turbopages.org/topnews-ru.ru/s/2022/08/15/djoan-royling-prigrozili-smertu-za-podderjky-ryshdi-ty-sledyushaia redirected to https://topnews-ru.ru/2022/08/15/djoan-royling-prigrozili-smertu-za-podderjky-ryshdi-ty-sledyushaia without any issues.

Test Case #2 (URL #2)

  • launch 1.44.95 Chromium: 106.0.5249.40 and restarted Brave
  • ensured that BraveDebounceStudy:Enabled via brave://version

Ensured that visiting https://08-mchs-gov-ru.turbopages.org/08.mchs.gov.ru/s/deyatelnost/press-centr/novosti/4726459 redirected to https://08.mchs.gov.ru/deyatelnost/press-centr/novosti/4726459 without any issues.

Test Case #3 - Regex (multi)

Example (Disabled) Example (Enabled)
Screenshot_20220921_225147_Brave Screenshot_20220921_225107_Brave

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 OS/Desktop privacy/debounce URL debouncer QA Pass - Android ARM QA Pass - Android Tab QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-All-Platforms QA/Yes release-notes/exclude
Projects
None yet
Milestone
1.44.x - Release
Development

Successfully merging a pull request may close this issue.

Allow multiple capture groups for debounce regex action brave/brave-core
6 participants
@stephendonner @kjozwiak @ShivanKaul @LaurenWags @brave-builds @MadhaviSeelam