New issue
Advanced search Search tips

Issue 865032 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Jul 23
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Missing setting to revoke popup/redirect exception

Project Member Reported by rsch...@chromium.org, Jul 18

Issue description

Issue is present in 69.0.3493.4, but works as expected in 68.0.3440.40. Hoping the QA team can help us out with a bisect here.

Repro:
0. Tip: Test this on a testing profile for easier recovery
1. Go to https://ndossougbe.github.io/web-sandbox/interventions/3p-redirect/
2. When the "Redirect blocked" infobar appears, click "Details" and then "Always allow"
3. Go to the site settings for this site (three dots menu -> Settings -> Site settings -> All sites -> https://ndossougbe.github.io

Expected: In Permissions there's an entry for "Pop-ups and redirects", and when clicked the user can choose Allow or Block.

Actual: "Pop-ups and redirects" is missing under Permissions.

 
Labels: Needs-Bisect
Requesting a bisect to narrow down the regression. Thank you!
Cc: chelamcherla@chromium.org
Labels: Needs-triage-Mobile Triaged-Mobile Needs-Feedback
Tested the issue on Android and observed that "Pop-ups and redirects" option is seen and on clicking it able to choose Allow or Block.

Steps Followed:
1. Launch chrome
2. Navigated to https://ndossougbe.github.io/web-sandbox/interventions/3p-redirect/
3. Clicked "Details" and selected "Always Allow" on "Redirect Blocked" Infobar
4. Navigated to site settings > All sites > selected  https://ndossougbe.github.io and able to see "Pop-ups and redirects" option

Chrome version:
69.0.3493.4 and 69.0.3495.2

OS:
Android 9.0.0

Android Devices:
Pixel 2 XL, Nexus 6p

@rschoen: Could you please confirm details of device and Android OS on which you are seeing this issue. Please let us know if there are any flags enabled?  Also please update chrome to latest canary and check the issue.

Thanks!
re: c2

Thanks chelamcherla@! I see the setting exist when accessed via Settings > *All sites* (I didn't know about this path!). 

With that, we can narrow down the issue area to: 
- When accessed via Settings > *Pop-ups and redirects*

See the attached image.

It's observed in Canary: 69.0.3496.0 as well.

Thanks for looking into the issue!
redirect settings.png
421 KB View Download
Labels: -Needs-Feedback -Needs-Bisect hasbisect-per-revision RegressedIn-69 Target-69 FoundIn-69
Owner: mar...@mwiacek.com
Status: Assigned (was: Untriaged)
Tested the issue in Android and able to reproduce the issue. 

Steps Followed:
1. Launch chrome
2. Navigated to https://ndossougbe.github.io/web-sandbox/interventions/3p-redirect/
3. Clicked "Details" and selected "Always Allow" on "Redirect Blocked" Infobar
4. Navigated to site settings > Popup & Redirects > selected  https://ndossougbe.github.io and observed missing of "Pop-ups and redirects" option

Chrome versions tested:
69.0.3486.0(Latest canary)

OS:
Android 9.0.0

Android Devices:
Pixel 2 XL

Using the per-revision bisect providing the bisect results,
Good Build - 66.0.3466.0
Bad Build - 66.0.3469.0

You are looking for a change made after 569205(GOOD), but before 569206(BAD).

CHANGELOG URL:
https://chromium.googlesource.com/chromium/src/+/efc4691f78d32a69af5da3067e3fe66aa128586f

@ marcin :  Could you please look into the issue, pardon me if it has nothing to do with your changes and if possible please assign it to owner concerned.

Please navigate to below link for log's  --
go/chrome-androidlogs/865032


Thanks!
I will look on it, thx for assigning.
++Correction: 

Good Build: 69.0.3466.0
Bad Build: 69.0.3469.0
Status: Started (was: Assigned)
Patch is waiting for review: https://chromium-review.googlesource.com/c/chromium/src/+/1145434
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 23

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/97d5a24069b68d4261144f69e0ed7695df32711d

commit 97d5a24069b68d4261144f69e0ed7695df32711d
Author: Marcin Wiacek <marcin@mwiacek.com>
Date: Mon Jul 23 10:26:54 2018

Fix for displaying ContentSettingException in one Settings screen path

This patch is fixing change made with the
https://chromium-review.googlesource.com/c/chromium/src/+/1097408.

Before patch: ContentSettingException permissions are not displayed.

After patch: ContentSettingException are visible.

Affected path: Settings->Site Settings->(setting)->(exception site)

BUG= 865032 

Change-Id: I35dffa8c071fae4f4e76c69850b542b747dc493e
Reviewed-on: https://chromium-review.googlesource.com/1145434
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Marcin Wiącek <marcin@mwiacek.com>
Cr-Commit-Position: refs/heads/master@{#577135}
[modify] https://crrev.com/97d5a24069b68d4261144f69e0ed7695df32711d/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java

Status: Fixed (was: Started)
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-69; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-69 label, otherwise remove Merge-TBD label. Thanks.
Labels: -Merge-TBD Merge-Request-69
According to my best knowledge merge is required, please help me with this.
Cc: benmason@chromium.org
Yes, since this is after 70 branch we should definitely merge to M69 branch as well. Expressing product support for the merge, but I don't know the technical details of how to actually merge :)
@marcin, I think you should be able to do the merge yourself once it's approved by following the Gerrit instructions on https://www.chromium.org/developers/how-tos/drover.
japhet@ thx a lot - I've created https://chromium-review.googlesource.com/c/chromium/src/+/1147160 and waiting for review from the same Reviewer like previously.

I hope that everything is done OK, feel free to make review and commit :)
Update: it needs approval from TPM, can you help me in getting it?
Update: waiting for approval, I've removed Reviewer from https://chromium-review.googlesource.com/c/chromium/src/+/1147160 
@marcin, the TPMs will see that this issue is flagged Merge-Request-69, and will replace that flag to Merge-Approved-69 when they approve it.
Labels: -Merge-Request-69 Merge-Approved-69
Approved for merge.
Thank you, I'm waiting for review and commit (it looks, I cannot do it with my access).
Have you made any further changes? You should be able to merge https://chromium-review.googlesource.com/c/chromium/src/+/1145434 into branch 3497.
benmason@, I need to make cherrypick from https://chromium-review.googlesource.com/c/chromium/src/+/1145434 and then commit.

Cherrypick was made (https://chromium-review.googlesource.com/c/chromium/src/+/1147160), but I cannot do dry run (no +1).

Sorry, but it looks, that my privileges are not enough, feel free to proceed if you can.
in #23 I wanted to say: "I cannot EVEN do dry run."

When bauerb@ is in tomorrow, he should be able to LGTM and submit it to the commit queue. I'll give him first chance at it, otherwise if he's not available tomorrow I'll do it.
thx.
oooo, it's merged.

magic.

thx benmason@
Project Member

Comment 28 by bugdroid1@chromium.org, Jul 23

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6307484921fb6bbf1d77488a26abcd512a612285

commit 6307484921fb6bbf1d77488a26abcd512a612285
Author: Marcin Wiacek <marcin@mwiacek.com>
Date: Mon Jul 23 20:22:12 2018

M69:fix for displaying ContentSettingException in 1 Settings screen path

This patch is fixing change made with the
https://chromium-review.googlesource.com/c/chromium/src/+/1097408.

Before patch: ContentSettingException permissions are not displayed.

After patch: ContentSettingException are visible.

Affected path: Settings->Site Settings->(setting)->(exception site)

BUG= 865032 

Change-Id: I35dffa8c071fae4f4e76c69850b542b747dc493e
Reviewed-on: https://chromium-review.googlesource.com/1145434
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Commit-Queue: Marcin Wiącek <marcin@mwiacek.com>
Cr-Original-Commit-Position: refs/heads/master@{#577135}(cherry picked from commit 97d5a24069b68d4261144f69e0ed7695df32711d)
Reviewed-on: https://chromium-review.googlesource.com/1147160
Reviewed-by: Ben Mason <benmason@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#26}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/6307484921fb6bbf1d77488a26abcd512a612285/chrome/android/java/src/org/chromium/chrome/browser/preferences/website/SingleWebsitePreferences.java

Status: Verified (was: Fixed)
Verified in 69.0.3497.9

Sign in to add a comment