Issue metadata
Sign in to add a comment
|
Missing setting to revoke popup/redirect exception |
||||||||||||||||||||||
Issue descriptionIssue 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.
,
Jul 19
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!
,
Jul 19
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!
,
Jul 20
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!
,
Jul 20
I will look on it, thx for assigning.
,
Jul 20
++Correction: Good Build: 69.0.3466.0 Bad Build: 69.0.3469.0
,
Jul 21
Patch is waiting for review: https://chromium-review.googlesource.com/c/chromium/src/+/1145434
,
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
,
Jul 23
,
Jul 23
[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.
,
Jul 23
,
Jul 23
According to my best knowledge merge is required, please help me with this.
,
Jul 23
,
Jul 23
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 :)
,
Jul 23
@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.
,
Jul 23
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 :)
,
Jul 23
Update: it needs approval from TPM, can you help me in getting it?
,
Jul 23
Update: waiting for approval, I've removed Reviewer from https://chromium-review.googlesource.com/c/chromium/src/+/1147160
,
Jul 23
@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.
,
Jul 23
Approved for merge.
,
Jul 23
Thank you, I'm waiting for review and commit (it looks, I cannot do it with my access).
,
Jul 23
Have you made any further changes? You should be able to merge https://chromium-review.googlesource.com/c/chromium/src/+/1145434 into branch 3497.
,
Jul 23
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.
,
Jul 23
in #23 I wanted to say: "I cannot EVEN do dry run."
,
Jul 23
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.
,
Jul 23
thx.
,
Jul 23
oooo, it's merged. magic. thx benmason@
,
Jul 23
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
,
Jul 24
Verified in 69.0.3497.9 |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by rsch...@chromium.org
, Jul 18