Desktop: Alter framebust / popup UI for unification of the setting |
||||
Issue descriptionSee go/popup-redirect-unification-ui for mocks. At a high level, we want to add a sticky setting for declining the framebust / tab-under intervention, and we're using the popup setting as our mechanism. This bug should cover the following desktop UIs: - Popup blocked omnibox pop-over - Framebust blocked omnibox pop-over - Site settings - Site details - Page info
,
May 8 2018
,
May 8 2018
Making changes in crrev.com/c/1047687. Including screenshots of those changes.
,
May 8 2018
,
May 9 2018
srahim@ - re:c3 PopupOmnibox.png, if there's one pop-up blocked (like the example screenshot), should we not have 's' (plural) or should we always say "Pop-ups blocked"?
,
May 9 2018
ericrobinson@ - Thanks for the screenshots and for implementing the feature! They're looking great! Is there any open UX question? I saw crrev comments, but was unsure where to look / whether it's been resolved or not. Please let me and srahim@ know. Thanks!
,
May 9 2018
srahim@, hwi@: I believe the open question is around one of the Mac variables, but nothing else has been raised at this point. Regarding the plurals, we could aim to fix, but this matches the previous behavior, where the message above was simply: "The following pop-ups were blocked on this page:" Given this is a list, the plural here doesn't seem egregious, and I don't know the interface, as it is, supports differentiating (would have to look into it).
,
May 10 2018
We can probably fix the pluralization in a followup. Josh's change will make it so that we have the context of "how many" items are being blocked at the time we set the title, so it should be straightforward once his change lands.
Eric: To see other examples of using plural strings, see base::GetPluralStringFUTF{8,16}.
,
May 10 2018
Attached is a screenshot of the new FrameBusting bubble incorporating Eric's string changes. This is for CL https://crrev.com/c/1050198.
,
May 10 2018
,
May 10 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fe3125b5bde03863132a92321377ef05b7e20cfa commit fe3125b5bde03863132a92321377ef05b7e20cfa Author: Eric Robinson <ericrobinson@chromium.org> Date: Thu May 10 18:04:51 2018 Changed wording for "pop-up"s blocked to include redirects on desktop. This changes the content of generated_resources.grd so that pop-up blocking content now referes to both pop-ups and redirects. Also removing a block of unused strings in generated_resources.grd. Bug: 840356 Change-Id: I3a8657ccd3188a1019327bd5499ed19db31899b7 Reviewed-on: https://chromium-review.googlesource.com/1047687 Commit-Queue: Eric Robinson <ericrobinson@chromium.org> Reviewed-by: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Scott Chen <scottchen@chromium.org> Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org> Cr-Commit-Position: refs/heads/master@{#557585} [modify] https://crrev.com/fe3125b5bde03863132a92321377ef05b7e20cfa/chrome/app/generated_resources.grd [modify] https://crrev.com/fe3125b5bde03863132a92321377ef05b7e20cfa/chrome/app/nibs/ContentBlockedPopups.xib [modify] https://crrev.com/fe3125b5bde03863132a92321377ef05b7e20cfa/chrome/app/settings_strings.grdp [modify] https://crrev.com/fe3125b5bde03863132a92321377ef05b7e20cfa/chrome/browser/ui/content_settings/content_setting_bubble_model.cc [modify] https://crrev.com/fe3125b5bde03863132a92321377ef05b7e20cfa/chrome/browser/ui/page_info/page_info_ui.cc
,
May 11 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fe6ffb824ee4043402aa4a6283fceb53b5996754 commit fe6ffb824ee4043402aa4a6283fceb53b5996754 Author: Josh Karlin <jkarlin@chromium.org> Date: Fri May 11 14:45:27 2018 Make the FrameBusting content setting bubble more like popups What: Changes the ContentSettingFramebustBlockBubbleModel to behave more like ContentSettingPopupBubbleModel. It now uses the same content type but differs in title and what it observes. Why: Popups and Framebusting behavior is merging. They'll use many of the same strings and they will share content setting storage. A screenshot of the new bubble is here: https://crbug.com/840356#c9 Bug: 840356 Change-Id: I098b0c3c7d94dcad58b2f01ebb02493503ecd835 Reviewed-on: https://chromium-review.googlesource.com/1050198 Commit-Queue: Josh Karlin <jkarlin@chromium.org> Reviewed-by: Bernhard Bauer <bauerb@chromium.org> Reviewed-by: Charlie Harrison <csharrison@chromium.org> Cr-Commit-Position: refs/heads/master@{#557877} [modify] https://crrev.com/fe6ffb824ee4043402aa4a6283fceb53b5996754/chrome/browser/ui/content_settings/content_setting_bubble_model.cc [modify] https://crrev.com/fe6ffb824ee4043402aa4a6283fceb53b5996754/chrome/browser/ui/content_settings/content_setting_bubble_model.h [modify] https://crrev.com/fe6ffb824ee4043402aa4a6283fceb53b5996754/chrome/browser/ui/content_settings/framebust_block_browsertest.cc
,
May 11 2018
,
May 14 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0b8c73cbd8430e61a8165d719c3ad2d076d5b983 commit 0b8c73cbd8430e61a8165d719c3ad2d076d5b983 Author: Eric Robinson <ericrobinson@chromium.org> Date: Mon May 14 02:45:20 2018 Removing unused string for pop-ups menu. The pop-up string has been replaced with pop-ups and redirects. I've removed the old string, which will no longer be referenced after crrev.com/c/1047687. Bug: 840356 Change-Id: Iaf65ce22e3296215f27d25dfa7b7a81b0c605758 Reviewed-on: https://chromium-review.googlesource.com/1053953 Reviewed-by: Patti <patricialor@chromium.org> Commit-Queue: Eric Robinson <ericrobinson@chromium.org> Cr-Commit-Position: refs/heads/master@{#558180} [modify] https://crrev.com/0b8c73cbd8430e61a8165d719c3ad2d076d5b983/components/page_info_strings.grdp |
||||
►
Sign in to add a comment |
||||
Comment 1 by csharrison@chromium.org
, May 7 2018