New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 689992 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug

Blocked on:
issue 689487
issue 707026
issue 708719

Blocking:
issue 609747



Sign in to add a comment

Tracking bug for the new UI for the full launch of the Subresource Filter.

Project Member Reported by melandory@chromium.org, Feb 8 2017

Issue description

Explanation for the current state of the UI and plans.

https://goto.google.com/subresource-filter-ui-explanation
(includes links to the mocks in Folio)
 
In attachment you can find screenshots of the current prototype for the Clank UI.
Screenshot_20170321-170405.png
1.4 MB View Download
Screenshot_20170322-104116.png
875 KB View Download
Description: Show this description
Labels: OS-Chrome
Status: Started (was: Available)
Cc: shivanisha@chromium.org
+shivanisha FYI
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 28 2017

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

commit 9b44551e3f90c894305a6ec32bc1ae1b7e8f3f21
Author: melandory <melandory@chromium.org>
Date: Tue Mar 28 13:54:48 2017

Prototype for the new UI for the Safe Browsing Subresource Filter.

This CL adds the prototype for the new UI for the Subresource Filter for
the Android platform. New UI consists of the small version of the
infobar and the "Details" link. When the link is pressed full infobar
with detailed text and the action buttons appears.

The bug contains screenshots for the UI implemeted in this CL and mocks.
Note that, this CL is the first step towards, but does not yet implement
the UI as in mocks.

BUG= 689992 

Review-Url: https://codereview.chromium.org/2765193002
Cr-Commit-Position: refs/heads/master@{#460085}

[add] https://crrev.com/9b44551e3f90c894305a6ec32bc1ae1b7e8f3f21/chrome/android/java/src/org/chromium/chrome/browser/infobar/SubresourceFilterExperimentalInfoBar.java
[modify] https://crrev.com/9b44551e3f90c894305a6ec32bc1ae1b7e8f3f21/chrome/android/java/src/org/chromium/chrome/browser/page_info/PageInfoPopup.java
[modify] https://crrev.com/9b44551e3f90c894305a6ec32bc1ae1b7e8f3f21/chrome/android/java/strings/android_chrome_strings.grd
[modify] https://crrev.com/9b44551e3f90c894305a6ec32bc1ae1b7e8f3f21/chrome/android/java_sources.gni
[modify] https://crrev.com/9b44551e3f90c894305a6ec32bc1ae1b7e8f3f21/chrome/browser/BUILD.gn
[modify] https://crrev.com/9b44551e3f90c894305a6ec32bc1ae1b7e8f3f21/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/9b44551e3f90c894305a6ec32bc1ae1b7e8f3f21/chrome/browser/ui/android/content_settings/subresource_filter_infobar_delegate.cc
[modify] https://crrev.com/9b44551e3f90c894305a6ec32bc1ae1b7e8f3f21/chrome/browser/ui/android/content_settings/subresource_filter_infobar_delegate.h
[modify] https://crrev.com/9b44551e3f90c894305a6ec32bc1ae1b7e8f3f21/chrome/browser/ui/android/infobars/subresource_filter_infobar.cc
[modify] https://crrev.com/9b44551e3f90c894305a6ec32bc1ae1b7e8f3f21/components/subresource_filter/core/browser/subresource_filter_features.cc
[modify] https://crrev.com/9b44551e3f90c894305a6ec32bc1ae1b7e8f3f21/components/subresource_filter/core/browser/subresource_filter_features.h

Project Member

Comment 6 by bugdroid1@chromium.org, Mar 29 2017

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

commit 7be36d31cf92f725808f0260e565ed093541505f
Author: melandory <melandory@chromium.org>
Date: Wed Mar 29 15:51:12 2017

Add desktop UI for the subresource filter content setting.

Added option on chrome://settings/content to toggle subresource filter on a
global or per-site basis.

BUG= 689487 ,  689992 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2682293002
Cr-Commit-Position: refs/heads/master@{#460402}

[modify] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/app/generated_resources.grd
[add] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/app/theme/default_100_percent/common/allowed_subresource_filter.png
[add] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/app/theme/default_100_percent/common/blocked_subresource_filter.png
[add] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/app/theme/default_200_percent/common/allowed_subresource_filter.png
[add] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/app/theme/default_200_percent/common/blocked_subresource_filter.png
[modify] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/app/theme/theme_resources.grd
[modify] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/browser/resources/settings/privacy_page/privacy_page.html
[modify] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/browser/resources/settings/privacy_page/privacy_page.js
[modify] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/browser/resources/settings/route.js
[modify] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/browser/resources/settings/site_settings/category_default_setting.js
[modify] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/browser/resources/settings/site_settings/constants.js
[modify] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/browser/resources/settings/site_settings_page/site_settings_page.html
[modify] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/browser/resources/settings/site_settings_page/site_settings_page.js
[modify] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/browser/ui/chrome_pages.cc
[modify] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/browser/ui/page_info/page_info.cc
[modify] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/browser/ui/page_info/page_info_ui.cc
[modify] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc
[modify] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/browser/ui/webui/site_settings_helper.cc
[modify] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/test/data/webui/settings/site_list_tests.js
[modify] https://crrev.com/7be36d31cf92f725808f0260e565ed093541505f/chrome/test/data/webui/settings/test_site_settings_prefs_browser_proxy.js

Blockedon: 707026
Cc: csharrison@chromium.org
Screenshots for the new toggle control for subresource filtering.
For real this time...
toggle_off_2.png
62.1 KB View Download
toggle_on_2.png
63.1 KB View Download

Comment 10 by msw@chromium.org, Apr 4 2017

Why wouldn't this just be another single button, like [Load Full Site] [OK] instead of a checkbox/toggle that alters the ok button? It seems like the mock in the document shows that two-button approach. This toggle is a bit weird.
Cc: chowse@chromium.org
+chowse, can you give context for why we thought the toggle/checkmark was a better experience than a separate button?
msw: Note that the strings after full launch make a bit more sense with a toggle / checkbox.

See go/subresource-filter-ui-explanation for links to mocks with better strings.
Cc: srahim@chromium.org
As charrison mentioned, the strings here are not final. The switch should eventually be labelled "Always show ads on this site", not "Load full site". +srahim

The intention of a switch vs. a button was two-fold:

1. To make clear this action is a permanent opt-out (like a permission). In other contexts where we use a "Load full site" button, its longevity is not clear, but is typically for the remainder of the session.

2. While we want to make disabling ad blocking straightforward, we don't want to make it the primary CTA. Having a button labelled "Always show ads on this site" would be enormous and the first thing people would see when expanding the infobar. We want the focus to be more on *why* ads were blocked, with an opt-out as a safety measure. Using a switch + button accomplished this better, though it adds a second tap.

Blockedon: 708719
Project Member

Comment 15 by bugdroid1@chromium.org, Apr 5 2017

Cc: msw@chromium.org
msw: does #13 address your concerns?

Comment 17 Deleted

Comment 18 by msw@chromium.org, Apr 5 2017

I guess so, looking at https://codereview.chromium.org/2793413002 now
Looks like the button resizing logic needs more love, this is what I am seeing on ToT on Linux.
overflow.png
46.3 KB View Download
Yes I have a CL in flight for this.
Project Member

Comment 22 by bugdroid1@chromium.org, Apr 10 2017

Project Member

Comment 24 by bugdroid1@chromium.org, Apr 12 2017

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

commit dda3a2b272de6426c2d1da121731d8acd016569f
Author: csharrison <csharrison@chromium.org>
Date: Wed Apr 12 20:08:00 2017

[subresource_filter] Don't regress the Mac UI

The Mac UI was not updated in [1] when it probably should have. To
ensure code does not stay broken past M59 branch point, just ensure that
the existing UI can co-exist with the new UI

[1]: https://codereview.chromium.org/2793413002
BUG= 689992 

Review-Url: https://codereview.chromium.org/2816593005
Cr-Commit-Position: refs/heads/master@{#464126}

[modify] https://crrev.com/dda3a2b272de6426c2d1da121731d8acd016569f/chrome/browser/ui/content_settings/content_setting_bubble_model.cc
[modify] https://crrev.com/dda3a2b272de6426c2d1da121731d8acd016569f/chrome/browser/ui/content_settings/content_setting_bubble_model.h

Project Member

Comment 25 by bugdroid1@chromium.org, Apr 24 2017

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

commit 97dde4e681d09822423e59c6a9dd7c5ece27283b
Author: csharrison <csharrison@chromium.org>
Date: Mon Apr 24 16:35:50 2017

Add SubresourceFilterProfileContext for all profile scoped context

This gives us an object to hang profile-scoped members, which will be
important if we ever need a broader scope for making policy decisions
than a WebContents-scope.

This CL changes no functionality.

BUG= 689992 

Review-Url: https://codereview.chromium.org/2829313002
Cr-Commit-Position: refs/heads/master@{#466655}

[modify] https://crrev.com/97dde4e681d09822423e59c6a9dd7c5ece27283b/chrome/browser/BUILD.gn
[modify] https://crrev.com/97dde4e681d09822423e59c6a9dd7c5ece27283b/chrome/browser/subresource_filter/chrome_subresource_filter_client.cc
[modify] https://crrev.com/97dde4e681d09822423e59c6a9dd7c5ece27283b/chrome/browser/subresource_filter/subresource_filter_content_settings_manager.cc
[modify] https://crrev.com/97dde4e681d09822423e59c6a9dd7c5ece27283b/chrome/browser/subresource_filter/subresource_filter_content_settings_manager.h
[delete] https://crrev.com/3b0deba895e763195d439c7fcd4d5f947509904d/chrome/browser/subresource_filter/subresource_filter_content_settings_manager_factory.cc
[delete] https://crrev.com/3b0deba895e763195d439c7fcd4d5f947509904d/chrome/browser/subresource_filter/subresource_filter_content_settings_manager_factory.h
[rename] https://crrev.com/97dde4e681d09822423e59c6a9dd7c5ece27283b/chrome/browser/subresource_filter/subresource_filter_content_settings_manager_unittest.cc
[add] https://crrev.com/97dde4e681d09822423e59c6a9dd7c5ece27283b/chrome/browser/subresource_filter/subresource_filter_profile_context.cc
[add] https://crrev.com/97dde4e681d09822423e59c6a9dd7c5ece27283b/chrome/browser/subresource_filter/subresource_filter_profile_context.h
[add] https://crrev.com/97dde4e681d09822423e59c6a9dd7c5ece27283b/chrome/browser/subresource_filter/subresource_filter_profile_context_factory.cc
[add] https://crrev.com/97dde4e681d09822423e59c6a9dd7c5ece27283b/chrome/browser/subresource_filter/subresource_filter_profile_context_factory.h
[modify] https://crrev.com/97dde4e681d09822423e59c6a9dd7c5ece27283b/chrome/test/BUILD.gn

screenshot from the MAC UI CL: https://codereview.chromium.org/2826233002
Screen Shot 2017-05-03 at 10.43.27 AM.png
46.6 KB View Download
screenshot from the MAC UI CL: https://codereview.chromium.org/2826233002
Screen Shot 2017-05-03 at 10.48.25 AM.png
47.8 KB View Download
Owner: csharrison@chromium.org
Project Member

Comment 30 by bugdroid1@chromium.org, May 5 2017

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

commit 48e975e533eaf4b951551df1ea01e90ab5930fd8
Author: csharrison <csharrison@chromium.org>
Date: Fri May 05 20:27:52 2017

[subresource_filter] Make placeholder strings untranslateable

These will never be shown to users.

BUG= 689992 

Review-Url: https://codereview.chromium.org/2863033002
Cr-Commit-Position: refs/heads/master@{#469765}

[modify] https://crrev.com/48e975e533eaf4b951551df1ea01e90ab5930fd8/chrome/android/java/strings/android_chrome_strings.grd
[modify] https://crrev.com/48e975e533eaf4b951551df1ea01e90ab5930fd8/chrome/app/generated_resources.grd

Project Member

Comment 31 by bugdroid1@chromium.org, May 8 2017

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

commit da2a48d9f7403a1fbee3a5984d442a49ef14a01c
Author: shivanisha <shivanisha@chromium.org>
Date: Mon May 08 14:16:35 2017

[subresource_filter] Mac UI updated and xib replaced with code based layout.
- Replaces xib based layout with code based layout.
- Updates the UI as per the latest mocks.

BUG= 689992 

Review-Url: https://codereview.chromium.org/2826233002
Cr-Commit-Position: refs/heads/master@{#469979}

[modify] https://crrev.com/da2a48d9f7403a1fbee3a5984d442a49ef14a01c/chrome/app/nibs/BUILD.gn
[delete] https://crrev.com/da7c77096c29ccd804917018dc74bebd925e164a/chrome/app/nibs/ContentSubresourceFilter.xib
[modify] https://crrev.com/da2a48d9f7403a1fbee3a5984d442a49ef14a01c/chrome/browser/ui/BUILD.gn
[modify] https://crrev.com/da2a48d9f7403a1fbee3a5984d442a49ef14a01c/chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.h
[modify] https://crrev.com/da2a48d9f7403a1fbee3a5984d442a49ef14a01c/chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa.mm
[modify] https://crrev.com/da2a48d9f7403a1fbee3a5984d442a49ef14a01c/chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa_browsertest.mm
[add] https://crrev.com/da2a48d9f7403a1fbee3a5984d442a49ef14a01c/chrome/browser/ui/cocoa/subresource_filter/subresource_filter_bubble_controller.h
[add] https://crrev.com/da2a48d9f7403a1fbee3a5984d442a49ef14a01c/chrome/browser/ui/cocoa/subresource_filter/subresource_filter_bubble_controller.mm

Project Member

Comment 32 by bugdroid1@chromium.org, May 8 2017

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

commit b081a87e2df7e28ae9cef236fa29357236f09d9e
Author: csharrison <csharrison@chromium.org>
Date: Mon May 08 16:44:56 2017

[subresource_filter] Remove dead OnManageLinkClicked code path

Now that the updated UI [1] on Mac has landed, this path is unused.

[1]: https://codereview.chromium.org/2826233002/

BUG= 689992 

Review-Url: https://codereview.chromium.org/2872633002
Cr-Commit-Position: refs/heads/master@{#470016}

[modify] https://crrev.com/b081a87e2df7e28ae9cef236fa29357236f09d9e/chrome/browser/ui/content_settings/content_setting_bubble_model.cc
[modify] https://crrev.com/b081a87e2df7e28ae9cef236fa29357236f09d9e/chrome/browser/ui/content_settings/content_setting_bubble_model.h

Project Member

Comment 33 by bugdroid1@chromium.org, May 9 2017

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

commit 3f8cdfd3ab1a3e87f90c37f3d7763483ac9e03c4
Author: csharrison <csharrison@chromium.org>
Date: Tue May 09 04:41:46 2017

[subresource_filter] Implement the "Smart" UI on Android

This patch implements a V1 for the "smart" UI, which just logs a
timestamp every time the UI is shown, and will not show the UI on
subsequent navigations if it is within 30 minutes of a previous impression.

This is implemented using a new WebsiteSetting, which holds a timestamp.

BUG= 689992 

Review-Url: https://codereview.chromium.org/2795053002
Cr-Commit-Position: refs/heads/master@{#470217}

[modify] https://crrev.com/3f8cdfd3ab1a3e87f90c37f3d7763483ac9e03c4/chrome/browser/subresource_filter/chrome_subresource_filter_client.cc
[modify] https://crrev.com/3f8cdfd3ab1a3e87f90c37f3d7763483ac9e03c4/chrome/browser/subresource_filter/chrome_subresource_filter_client.h
[modify] https://crrev.com/3f8cdfd3ab1a3e87f90c37f3d7763483ac9e03c4/chrome/browser/subresource_filter/subresource_filter_browsertest.cc
[modify] https://crrev.com/3f8cdfd3ab1a3e87f90c37f3d7763483ac9e03c4/chrome/browser/subresource_filter/subresource_filter_content_settings_manager.cc
[modify] https://crrev.com/3f8cdfd3ab1a3e87f90c37f3d7763483ac9e03c4/chrome/browser/subresource_filter/subresource_filter_content_settings_manager.h
[modify] https://crrev.com/3f8cdfd3ab1a3e87f90c37f3d7763483ac9e03c4/chrome/browser/subresource_filter/subresource_filter_content_settings_manager_unittest.cc
[modify] https://crrev.com/3f8cdfd3ab1a3e87f90c37f3d7763483ac9e03c4/chrome/browser/subresource_filter/subresource_filter_profile_context.cc
[modify] https://crrev.com/3f8cdfd3ab1a3e87f90c37f3d7763483ac9e03c4/chrome/browser/subresource_filter/subresource_filter_profile_context.h
[modify] https://crrev.com/3f8cdfd3ab1a3e87f90c37f3d7763483ac9e03c4/chrome/browser/subresource_filter/subresource_filter_profile_context_factory.cc
[modify] https://crrev.com/3f8cdfd3ab1a3e87f90c37f3d7763483ac9e03c4/chrome/browser/subresource_filter/subresource_filter_profile_context_factory.h
[modify] https://crrev.com/3f8cdfd3ab1a3e87f90c37f3d7763483ac9e03c4/components/content_settings/core/browser/website_settings_registry.cc
[modify] https://crrev.com/3f8cdfd3ab1a3e87f90c37f3d7763483ac9e03c4/components/content_settings/core/common/content_settings.cc
[modify] https://crrev.com/3f8cdfd3ab1a3e87f90c37f3d7763483ac9e03c4/components/content_settings/core/common/content_settings_types.h
[modify] https://crrev.com/3f8cdfd3ab1a3e87f90c37f3d7763483ac9e03c4/components/subresource_filter/content/browser/subresource_filter_client.h
[modify] https://crrev.com/3f8cdfd3ab1a3e87f90c37f3d7763483ac9e03c4/tools/metrics/histograms/enums.xml

Labels: -Restrict-View-Google
Icon change snapshot for desktop (CL:https://codereview.chromium.org/2926113003)
Screenshot from 2017-06-07 12:23:31.png
2.1 KB View Download
Icon change snapshot for Android (CL: https://codereview.chromium.org/2930753002)
Screenshot from 2017-06-08 13:31:28.png
58.1 KB View Download
New UI on mac.
mac_bubble.png
249 KB View Download
Screenshot for adding a banner on site details page.
Screenshot from 2017-06-12 16:48:46.png
50.5 KB View Download
The banner with the updated string is attached.
Screenshot from 2017-06-15 14:21:47.png
41.8 KB View Download
Project Member

Comment 41 by bugdroid1@chromium.org, Jun 15 2017

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

commit a18f38c607db22eed956aff833f70ad246bc6ea6
Author: Charles Harrison <csharrison@chromium.org>
Date: Thu Jun 15 22:22:35 2017

[subresource_filter] Replace the infobar with the experimental infobar

This CL replaces the subresource filter infobar with an "AdsBlockedInfobar"
and updates strings accordingly. This infobar will be used on master for
filtering on pages just after a safe browsing interstitial. Other uses
(e.g. for blocking more generally on sites which show intrusive ads)
is still behind a flag

Bug:  689992 
Change-Id: I62bf99e3f67e4a32fec8820f5bdad0b93da3d17e
Reviewed-on: https://chromium-review.googlesource.com/526092
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Dan Alcantara <dfalcantara@chromium.org>
Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#479864}
[rename] https://crrev.com/a18f38c607db22eed956aff833f70ad246bc6ea6/chrome/android/java/src/org/chromium/chrome/browser/infobar/AdsBlockedInfoBar.java
[delete] https://crrev.com/db6d7fe9267ec2297918e2589558f0a46d1f7039/chrome/android/java/src/org/chromium/chrome/browser/infobar/SubresourceFilterInfoBar.java
[modify] https://crrev.com/a18f38c607db22eed956aff833f70ad246bc6ea6/chrome/android/java_sources.gni
[modify] https://crrev.com/a18f38c607db22eed956aff833f70ad246bc6ea6/chrome/app/generated_resources.grd
[modify] https://crrev.com/a18f38c607db22eed956aff833f70ad246bc6ea6/chrome/browser/BUILD.gn
[modify] https://crrev.com/a18f38c607db22eed956aff833f70ad246bc6ea6/chrome/browser/subresource_filter/chrome_subresource_filter_client.cc
[modify] https://crrev.com/a18f38c607db22eed956aff833f70ad246bc6ea6/chrome/browser/ui/BUILD.gn
[add] https://crrev.com/a18f38c607db22eed956aff833f70ad246bc6ea6/chrome/browser/ui/android/content_settings/ads_blocked_infobar_delegate.cc
[add] https://crrev.com/a18f38c607db22eed956aff833f70ad246bc6ea6/chrome/browser/ui/android/content_settings/ads_blocked_infobar_delegate.h
[delete] https://crrev.com/db6d7fe9267ec2297918e2589558f0a46d1f7039/chrome/browser/ui/android/content_settings/subresource_filter_infobar_delegate.cc
[delete] https://crrev.com/db6d7fe9267ec2297918e2589558f0a46d1f7039/chrome/browser/ui/android/content_settings/subresource_filter_infobar_delegate.h
[add] https://crrev.com/a18f38c607db22eed956aff833f70ad246bc6ea6/chrome/browser/ui/android/infobars/ads_blocked_infobar.cc
[add] https://crrev.com/a18f38c607db22eed956aff833f70ad246bc6ea6/chrome/browser/ui/android/infobars/ads_blocked_infobar.h
[delete] https://crrev.com/db6d7fe9267ec2297918e2589558f0a46d1f7039/chrome/browser/ui/android/infobars/subresource_filter_infobar.cc
[delete] https://crrev.com/db6d7fe9267ec2297918e2589558f0a46d1f7039/chrome/browser/ui/android/infobars/subresource_filter_infobar.h
[modify] https://crrev.com/a18f38c607db22eed956aff833f70ad246bc6ea6/components/infobars/core/infobar_delegate.h
[modify] https://crrev.com/a18f38c607db22eed956aff833f70ad246bc6ea6/tools/metrics/histograms/enums.xml

Project Member

Comment 42 by bugdroid1@chromium.org, Jun 16 2017

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

commit 92301b9f6887c6602f27bcf34836938cd6b3c9ce
Author: Charles Harrison <csharrison@chromium.org>
Date: Fri Jun 16 14:27:52 2017

[subresource_filter] s/show/allow/ in the UI

Per discussion, these are better strings to show to users

Bug:  689992 
Change-Id: Ibab94253f34d2d149922611270862f4bef0d8b07
Reviewed-on: https://chromium-review.googlesource.com/538212
Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#480037}
[modify] https://crrev.com/92301b9f6887c6602f27bcf34836938cd6b3c9ce/chrome/app/generated_resources.grd
[modify] https://crrev.com/92301b9f6887c6602f27bcf34836938cd6b3c9ce/chrome/browser/ui/android/content_settings/ads_blocked_infobar_delegate.cc

Project Member

Comment 43 by bugdroid1@chromium.org, Jun 16 2017

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

commit 544d4d0467a1eedab9469406fb40cc89778f948f
Author: Charles Harrison <csharrison@chromium.org>
Date: Fri Jun 16 17:02:45 2017

[subresource_filter] Desktop UI changes

The patch updates the strings in the desktop UI, and removes some
old unnecessary ones.

Additionally, there is logic to remove padding from an infobar if
it contains no title.

Bug:  689992 
Change-Id: I6f6f28e32a684a8ffffbac1afc22f168c242ef23
Reviewed-on: https://chromium-review.googlesource.com/527554
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Shivani Sharma <shivanisha@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#480079}
[modify] https://crrev.com/544d4d0467a1eedab9469406fb40cc89778f948f/chrome/app/generated_resources.grd
[modify] https://crrev.com/544d4d0467a1eedab9469406fb40cc89778f948f/chrome/browser/ui/android/content_settings/ads_blocked_infobar_delegate.cc
[modify] https://crrev.com/544d4d0467a1eedab9469406fb40cc89778f948f/chrome/browser/ui/cocoa/content_settings/content_setting_bubble_cocoa_browsertest.mm
[modify] https://crrev.com/544d4d0467a1eedab9469406fb40cc89778f948f/chrome/browser/ui/cocoa/subresource_filter/subresource_filter_bubble_controller.h
[modify] https://crrev.com/544d4d0467a1eedab9469406fb40cc89778f948f/chrome/browser/ui/cocoa/subresource_filter/subresource_filter_bubble_controller.mm
[modify] https://crrev.com/544d4d0467a1eedab9469406fb40cc89778f948f/chrome/browser/ui/content_settings/content_setting_bubble_model.cc
[modify] https://crrev.com/544d4d0467a1eedab9469406fb40cc89778f948f/chrome/browser/ui/content_settings/content_setting_bubble_model_unittest.cc
[modify] https://crrev.com/544d4d0467a1eedab9469406fb40cc89778f948f/chrome/browser/ui/content_settings/content_setting_image_model.cc
[modify] https://crrev.com/544d4d0467a1eedab9469406fb40cc89778f948f/chrome/browser/ui/views/content_setting_bubble_contents.cc

Icon updated for content settings page.
Screenshot from 2017-06-20 14:46:42.png
14.2 KB View Download
Screenshots for the "Learn more" links on mobile and desktop.
learn_more_mobile.png
188 KB View Download
learn_more_desktop.png
66.2 KB View Download
And Mac learn more link
mac_learn_more.png
226 KB View Download
Screenshot of the "learn more" link on Linux, with the learn_more_link code rather than the custom_link code.
learn_more_desktop_right.png
81.8 KB View Download
Screenshots for page info UI on desktop.
Screenshot from 2017-06-21 19:02:20.png
114 KB View Download
Screenshot from 2017-06-21 19:00:10.png
114 KB View Download
"flash blocked" UI for context for the "learn more" link. Note that this UI seems completely broken on Mac (with UMA to back it up - no one clicks learn more).
flash_blocked_dev.png
37.2 KB View Download
Project Member

Comment 51 by bugdroid1@chromium.org, Jun 22 2017

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

commit bd1d07ad2d804d305233a329e06ab74076098a34
Author: Charles Harrison <csharrison@chromium.org>
Date: Thu Jun 22 19:48:59 2017

[subresource_filter] Add Learn more link on Android

This patch adds a "Learn more" link to the expanded infobar, which will
send users to a Chrome help page about the feature.

The page is not currently implemented, so until M61 hits stable the
link will be broken and just land on the support home page.

Bug:  689992 
Change-Id: If7d85ad643ce872a48fb7538967886ffebd5fe8d
Reviewed-on: https://chromium-review.googlesource.com/541519
Reviewed-by: Matthew Jones <mdjones@chromium.org>
Reviewed-by: Dan Alcantara <dfalcantara@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#481631}
[modify] https://crrev.com/bd1d07ad2d804d305233a329e06ab74076098a34/chrome/android/java/src/org/chromium/chrome/browser/infobar/AdsBlockedInfoBar.java
[modify] https://crrev.com/bd1d07ad2d804d305233a329e06ab74076098a34/chrome/browser/ui/android/content_settings/ads_blocked_infobar_delegate.cc
[modify] https://crrev.com/bd1d07ad2d804d305233a329e06ab74076098a34/chrome/browser/ui/android/content_settings/ads_blocked_infobar_delegate.h
[modify] https://crrev.com/bd1d07ad2d804d305233a329e06ab74076098a34/components/subresource_filter/core/browser/subresource_filter_constants.h

Screenshots attached with the content settings UI with the icon as received from bettes@
Screenshot from 2017-06-27 14:55:56.png
11.9 KB View Download
Screenshot from 2017-06-27 14:55:29.png
14.2 KB View Download
Updated icon screenshots for content settings.
Screenshot from 2017-06-27 19:39:37.png
11.3 KB View Download
Screenshot from 2017-06-27 19:40:10.png
14.1 KB View Download
lgtm
Project Member

Comment 58 by bugdroid1@chromium.org, Jun 28 2017

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

commit 6e26093ba629b7aa843f7284c8751d1f752f4b1e
Author: shivanisha <shivanisha@chromium.org>
Date: Wed Jun 28 03:40:42 2017

Updated Ads icon for content settings.
Screenshot attached in comment#56 in the associated bug.

BUG= 689992 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2947913003
Cr-Commit-Position: refs/heads/master@{#482873}

[modify] https://crrev.com/6e26093ba629b7aa843f7284c8751d1f752f4b1e/chrome/browser/resources/settings/icons.html
[modify] https://crrev.com/6e26093ba629b7aa843f7284c8751d1f752f4b1e/chrome/browser/resources/settings/site_settings_page/site_settings_page.html

Status: Fixed (was: Started)

Sign in to add a comment