Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1963203002:
[Chrome Settings UI] Show overruled User Exceptions as strike-through. (Closed)
|
Created:
4 years, 7 months ago by huangs Modified:
4 years, 6 months ago CC:
chromium-reviews, dbeam+watch-options_chromium.org, msramek+watch_chromium.org, michaelpg+watch-options_chromium.org, raymes+watch_chromium.org, arv+watch_chromium.org, markusheintz_, grt (UTC plus 2), Georges Khalil, ainslie Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[Chrome Settings UI] Show ineffective User Exceptions as strike-through.
If policy defaults (or other defaults) overrule exceptions, changing UI
to show the exceptions texts in strike-through style.
- Adding policy icons that display text bubble on click.
- Fixing existing text misalignment in "Behavior" column for
Policy or Extension Exceptions.
This replaces the earlier (buggy) attempt http://crrev.com/1855393006/
BUG=568031
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
Committed: https://crrev.com/f99222b2b909b00abc34e874a05201de35071737
Cr-Commit-Position: refs/heads/master@{#396914}
Patch Set 1 #Patch Set 2 : Also strike-through settings; using suggestions pop-up; simplify code. #Patch Set 3 : Sync. #Patch Set 4 : Change 'Behavior' colunm text sizing to fix alignment. #
Total comments: 19
Patch Set 5 : Fix typo. #
Total comments: 2
Patch Set 6 : Move common code to appendIndicatorElement(). #
Total comments: 12
Patch Set 7 : Update CSS and .overruled style usage. #Patch Set 8 : Sync. #
Messages
Total messages: 32 (9 generated)
Description was changed from ========== [Chrome Settings UI] Show ineffective User Exceptions as strike-through. If policy defaults (or other defaults) make user exceptions ineffective, display the exceptions in strike through style. Also adding hover text. This replaces the earlier (buggy) attempt http://crrev.com/1855393006/ BUG=568031 ========== to ========== [Chrome Settings UI] Show ineffective User Exceptions as strike-through. If policy defaults (or other defaults) make user exceptions ineffective, display the exceptions in strike through style. Also adding hover text. This replaces the earlier (buggy) attempt http://crrev.com/1855393006/ BUG=568031 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
huangs@chromium.org changed reviewers: + dbeam@chromium.org, estade@chromium.org
PTAL. I also sent out a short design doc with screenshots.
not lgtm who did UX for this? we use line-through very seldomly in Chrome. how do I discover the reason something is struck with tap, keyboard, or a screen reader?
Per discussion, UX review is underway. First round of feedback specifies that we should replace title="..." with the usual pop-up. That is implemented in Patch Set 2. (testing not lgtm)
Description was changed from ========== [Chrome Settings UI] Show ineffective User Exceptions as strike-through. If policy defaults (or other defaults) make user exceptions ineffective, display the exceptions in strike through style. Also adding hover text. This replaces the earlier (buggy) attempt http://crrev.com/1855393006/ BUG=568031 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== [Chrome Settings UI] Show ineffective User Exceptions as strike-through. If policy defaults (or other defaults) overrule exceptions, changing UI to show the exceptions texts in strike-through style. Also adding policy icons that display text bubble on click. This replaces the earlier (buggy) attempt http://crrev.com/1855393006/ BUG=568031 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
Got UI review approval from ainslie@, also fixing existing misalignment for "Behavior" item in Policy Exception / Extension Exception row (yellow background). PTAL.
Description was changed from ========== [Chrome Settings UI] Show ineffective User Exceptions as strike-through. If policy defaults (or other defaults) overrule exceptions, changing UI to show the exceptions texts in strike-through style. Also adding policy icons that display text bubble on click. This replaces the earlier (buggy) attempt http://crrev.com/1855393006/ BUG=568031 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== [Chrome Settings UI] Show ineffective User Exceptions as strike-through. If policy defaults (or other defaults) overrule exceptions, changing UI to show the exceptions texts in strike-through style. - Adding policy icons that display text bubble on click. - Fixing existing text misalignment in "Behavior" column for Policy or Extension Exceptions. This replaces the earlier (buggy) attempt http://crrev.com/1855393006/ BUG=568031 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
huangs@chromium.org changed reviewers: + bauerb@chromium.org
Ping review for JS/CSS. OWNER review to bauerb@ for all files.
I wonder whether we should find out about overruled exceptions by looking for inversions in the (prioritized) list of content setting rules. Basically, for a single given content setting provider, we order rules by specificity, with the more specific rule taking precedence over the less specific rule. But because we look at providers in order of their precedence, a general rule in a high-precedence provider could come before a specific rule in a low-precedence provider, so it would never have an effect. It should be possible to look through the list of rules to find such a situation and pass that to the Web UI. https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... File chrome/browser/resources/options/content_settings.css (right): https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... chrome/browser/resources/options/content_settings.css:38: .overruled span.exception-setting { Why is the span selector necessary? https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... chrome/browser/resources/options/content_settings.css:124: y Remove https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... File chrome/browser/resources/options/content_settings_exceptions_area.js (right): https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... chrome/browser/resources/options/content_settings_exceptions_area.js:366: this.appendChild(indicator); What happens if this is called multiple times (e.g. if content settings change)? Would we just append indicators? https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... chrome/browser/resources/options/content_settings_exceptions_area.js:551: if (item.dataItem.source == 'preference') { Doesn't this mean that any preference rule is marked as overruled by any higher-level rule, even if the higher-level rule doesn't apply to the same origins? E.g., if an extension sets a content setting for google.com, a preferece rule for yahoo.com would show up as overruled?
Changing the rule matching mechanism to prefer specific over general, across the
different providers would be a change beyond the sope of this CL. But for
discussion sake, we should also worry about "allow" vs. "blocked" vs. other. I
guess what's relevant to this CL is that we're duplicating logic of {"Policy
Default", "Exeption Default"} overruling User Exception. The routine I deleted
AreUserExceptionsAllowedForType()
would make control more centralizezd, but it creates extra rendering roundtrip.
However, we can also inject the information when exceptions change. This is
omitted for simplicity, but please let me know if you want this. Thanks!
https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/options/content_settings.css (right):
https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource...
chrome/browser/resources/options/content_settings.css:38: .overruled
span.exception-setting {
If we do don't do this, then "select.exception-setting" is included, and when we
edit an exception the selected item drop down list will have the strike-through
always. If we do that then the <select> options and the text box should get it
also. I think that seeing strike-through while editing would be considered
annoying -- but please propose this in the UI review if you'd prefer that.
https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource...
chrome/browser/resources/options/content_settings.css:124: y
On 2016/05/18 09:55:13, Bernhard Bauer wrote:
> Remove
Done.
https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource...
File chrome/browser/resources/options/content_settings_exceptions_area.js
(right):
https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource...
chrome/browser/resources/options/content_settings_exceptions_area.js:366:
this.appendChild(indicator);
If content settings change then the entire list gets regenerated. However,
relying on call patterns seem kinda fragile. Please confirm you want
unification code, then I'll add it (extra unused complexity to be defensive).
https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource...
chrome/browser/resources/options/content_settings_exceptions_area.js:551: if
(item.dataItem.source == 'preference') {
The "overruled" concept only applies to Policy Defaults and Extension Defaults.
We don't count exception overrides (inlcuding wildcards).
Correction: the "redundant logic" already existed in JS and I'm just continuing to use it. So I'd prefer not complicate things and keep the "overruled" logic the way it is in the CL.
PTAL.
On 2016/05/18 16:44:31, huangs wrote: > Correction: the "redundant logic" already existed in JS and I'm just continuing > to use it. So I'd prefer not complicate things and keep the "overruled" logic > the way it is in the CL. Sorry, what does that refer to? :) https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... File chrome/browser/resources/options/content_settings.css (right): https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... chrome/browser/resources/options/content_settings.css:38: .overruled span.exception-setting { On 2016/05/18 13:52:49, huangs wrote: > If we do don't do this, then "select.exception-setting" is included, and when we > edit an exception the selected item drop down list will have the strike-through > always. If we do that then the <select> options and the text box should get it > also. I think that seeing strike-through while editing would be considered > annoying -- but please propose this in the UI review if you'd prefer that. Hm... can we use an additional class for this? Otherwise it might inadvertently apply to another <span> element as well if we add one. https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... File chrome/browser/resources/options/content_settings_exceptions_area.js (right): https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... chrome/browser/resources/options/content_settings_exceptions_area.js:208: var indicator = new ControlledSettingIndicator(); This appears to duplicate the code you added. Could we extract this into a shared method? https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... chrome/browser/resources/options/content_settings_exceptions_area.js:551: if (item.dataItem.source == 'preference') { On 2016/05/18 13:52:49, huangs wrote: > The "overruled" concept only applies to Policy Defaults and Extension Defaults. > We don't count exception overrides (inlcuding wildcards). I don't really want to get into a product discussion, but... why don't we? Historically, we tried not to treat default settings vastly different from exceptions (default settings are just rules with wildcard patterns). https://codereview.chromium.org/1963203002/diff/80001/chrome/browser/resource... File chrome/browser/resources/options/content_settings_exceptions_area.js (right): https://codereview.chromium.org/1963203002/diff/80001/chrome/browser/resource... chrome/browser/resources/options/content_settings_exceptions_area.js:359: var indicator = new ControlledSettingIndicator(); This duplicates a lot of code in lines 208ff. Could we extract that into a shared method? (Also, the whole approach of using a fake Event seems a bit involved to me, when we could just call a method directly, but that's maybe not for now.)
Updated, PTAL. Branch point is coming, but I think I'll merge it into Beta? On 2016/05/19 14:20:41, Bernhard Bauer wrote: > On 2016/05/18 16:44:31, huangs wrote: > > Correction: the "redundant logic" already existed in JS and I'm just > continuing > > to use it. So I'd prefer not complicate things and keep the "overruled" logic > > the way it is in the CL. > > Sorry, what does that refer to? :) I'm referring to the fact that "policy" and "extension" overrules user exceptions: This is done in C++ via ProviderType ordering, and then enforced independently in JS UI. It would be nice to have one central place for this decision, and the consequences (or copies) of this ordering is propagated to UI. I was initially worried that I made things worse, but then realized I just reuse existing JS "duplicate" code, so I guess it's okay. :) https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... File chrome/browser/resources/options/content_settings.css (right): https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... chrome/browser/resources/options/content_settings.css:38: .overruled span.exception-setting { This is applied to ".overruled span.exception-setting", i.e., we'd have <some-tag class="overruled"> <span class="exception-settings">I have line-through</span> <span>I have normal style</span> </some-tag> So sibling <span> that doesn't have class="exception-settings" will be unaffected. Unless you mean something else? https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... File chrome/browser/resources/options/content_settings_exceptions_area.js (right): https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... chrome/browser/resources/options/content_settings_exceptions_area.js:208: var indicator = new ControlledSettingIndicator(); On 2016/05/19 14:20:40, Bernhard Bauer wrote: > This appears to duplicate the code you added. Could we extract this into a > shared method? Done. https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... chrome/browser/resources/options/content_settings_exceptions_area.js:551: if (item.dataItem.source == 'preference') { I think treating default as "super wildcard exception" would belong in the "model" layer. However, we separate these in the UI, so having a separate treatment would belong in the "view" and "controller" layer, so maybe there's no conflict fundamentally? https://codereview.chromium.org/1963203002/diff/80001/chrome/browser/resource... File chrome/browser/resources/options/content_settings_exceptions_area.js (right): https://codereview.chromium.org/1963203002/diff/80001/chrome/browser/resource... chrome/browser/resources/options/content_settings_exceptions_area.js:359: var indicator = new ControlledSettingIndicator(); Done; extracted to appendIndicatorElement(). Yeah the usage of event is yucky. Actually I think that the new routine should be a static function of ControlledSettingIndicator that injects a new instance of itself to a given element. Then it can check for existing instance and possibly replacing it. This also allows later cleanup re. fake event, without affecting callers. However, doing this cleanup probably does not belong in this CL.
LGTM if you desperately want to merge this into M52, but see my note below about buggyness (which might very well affect whether you'll be able to merge this), and you'll also need Dan's explicit approval, as he has N-O-L-G-T-M'd it. https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... File chrome/browser/resources/options/content_settings.css (right): https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... chrome/browser/resources/options/content_settings.css:38: .overruled span.exception-setting { On 2016/05/20 15:16:46, huangs wrote: > This is applied to ".overruled span.exception-setting", i.e., we'd have > > <some-tag class="overruled"> > <span class="exception-settings">I have line-through</span> > <span>I have normal style</span> > </some-tag> > > So sibling <span> that doesn't have class="exception-settings" will be > unaffected. Unless you mean something else? Mostly I'd be concerned that <span> is too narrow if we'd ever use a different tag for the display label. But I guess we only have two types right now, and we can cross that bridge when we come to it. https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... File chrome/browser/resources/options/content_settings_exceptions_area.js (right): https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... chrome/browser/resources/options/content_settings_exceptions_area.js:551: if (item.dataItem.source == 'preference') { On 2016/05/20 15:16:46, huangs wrote: > I think treating default as "super wildcard exception" would belong in the > "model" layer. However, we separate these in the UI, so having a separate > treatment would belong in the "view" and "controller" layer, so maybe there's no > conflict fundamentally? I'm not sure what you are trying to say. This UI will not in all cases where a rule does not have any effect show it as overruled, which I would consider a bug. It's true that the UI for a default setting looks different from the UI for an exception, but a) that is only the case for user-defined content settings, and this is for policy- or extension-defined content settings, and b) the question of whether a rule is effective or not (which is what we want to show here) is independent of the UI treatment. I mean, if you want to land this, I won't stop you, but like I said above, to me this code is not free of bugs, and I would like to see them addressed at least in the future.
Please see comments. Re. "bug", I'd argue it's a case of "incomplete fix"? Ping dbeam@, PTAL. https://chromiumcodereview.appspot.com/1963203002/diff/60001/chrome/browser/r... File chrome/browser/resources/options/content_settings.css (right): https://chromiumcodereview.appspot.com/1963203002/diff/60001/chrome/browser/r... chrome/browser/resources/options/content_settings.css:38: .overruled span.exception-setting { On 2016/05/23 13:34:11, Bernhard Bauer wrote: > On 2016/05/20 15:16:46, huangs wrote: > > This is applied to ".overruled span.exception-setting", i.e., we'd have > > > > <some-tag class="overruled"> > > <span class="exception-settings">I have line-through</span> > > <span>I have normal style</span> > > </some-tag> > > > > So sibling <span> that doesn't have class="exception-settings" will be > > unaffected. Unless you mean something else? > > Mostly I'd be concerned that <span> is too narrow if we'd ever use a different > tag for the display label. But I guess we only have two types right now, and we > can cross that bridge when we come to it. Acknowledged. https://chromiumcodereview.appspot.com/1963203002/diff/60001/chrome/browser/r... File chrome/browser/resources/options/content_settings_exceptions_area.js (right): https://chromiumcodereview.appspot.com/1963203002/diff/60001/chrome/browser/r... chrome/browser/resources/options/content_settings_exceptions_area.js:551: if (item.dataItem.source == 'preference') { By "bug" do you mean: (1) Code is not doing what is specified, or (2) The specification is wrong? My take: The specification is "A User Exception is 'overruled' if it becomes ineffective due to Policy Default or Extension Default settings. For 'overruled' user exception, change UI to draw line-through.". Are you saying that the spec should be: "A User Exception is 'overruled' if anything, i.e., {Policy, Extension} {Default, Exceptions} make it ineffective. For 'overruled' user exception, change UI to..." If so, then we are disagreeing on (2), and so I'd think "bug" is too strong; perhaps "incomplete fix"? If you mean something else, then sorry for misunderstanding! I'd like to sort this out before proceeding.
https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... File chrome/browser/resources/options/content_settings_exceptions_area.js (right): https://codereview.chromium.org/1963203002/diff/60001/chrome/browser/resource... chrome/browser/resources/options/content_settings_exceptions_area.js:551: if (item.dataItem.source == 'preference') { On 2016/05/24 05:02:00, huangs wrote: > By "bug" do you mean: > > (1) Code is not doing what is specified, or > (2) The specification is wrong? > > My take: The specification is "A User Exception is 'overruled' if it becomes > ineffective due to Policy Default or Extension Default settings. For > 'overruled' user exception, change UI to draw line-through.". > > Are you saying that the spec should be: "A User Exception is 'overruled' if > anything, i.e., {Policy, Extension} {Default, Exceptions} make it ineffective. > For 'overruled' user exception, change UI to..." Yes, that's what I mean. > If so, then we are disagreeing on (2), and so I'd think "bug" is too strong; > perhaps "incomplete fix"? What's the difference? I mean, I agree that this change is better than nothing (which is why I'm not objecting to it in principle), but the part not fixed by an incomplete fix would be a bug, no? > If you mean something else, then sorry for misunderstanding! I'd like to sort > this out before proceeding. >
Okay, as long as the meaning is conveyed, wording is just a label. For a complete fix, I'll draft a separate doc for discussion. My feel for this is that this work will be complexity without impact (this is a corner case, plus chrome://ml-settings will chrome://settings). As fun as coding this will be, I don't want be burnt by this issue again. Meanwhile planning on paper won't take long; I'll go ahead with the doc so reach consensus.
https://chromiumcodereview.appspot.com/1963203002/diff/100001/chrome/browser/... File chrome/browser/resources/options/content_settings.css (right): https://chromiumcodereview.appspot.com/1963203002/diff/100001/chrome/browser/... chrome/browser/resources/options/content_settings.css:27: width: 112px; how is this width calculated? https://chromiumcodereview.appspot.com/1963203002/diff/100001/chrome/browser/... chrome/browser/resources/options/content_settings.css:35: width: 96px; same question https://chromiumcodereview.appspot.com/1963203002/diff/100001/chrome/browser/... chrome/browser/resources/options/content_settings.css:39: text-decoration: line-through; why are there 2 rules that apply the same line-through style rather than 1? https://chromiumcodereview.appspot.com/1963203002/diff/100001/chrome/browser/... File chrome/browser/resources/options/content_settings_exceptions_area.js (right): https://chromiumcodereview.appspot.com/1963203002/diff/100001/chrome/browser/... chrome/browser/resources/options/content_settings_exceptions_area.js:243: var indicator = new ControlledSettingIndicator(); can't you just do indicator.controlledBy = controlledBy; instead of this new Event() + handlePrefChange() dance? https://chromiumcodereview.appspot.com/1963203002/diff/100001/chrome/browser/... chrome/browser/resources/options/content_settings_exceptions_area.js:248: event.value = { controlledBy: controlledBy }; no spaces between curlies: { controlledBy: controlledBy } ^ ^ should be: {controlledBy: controlledBy} https://chromiumcodereview.appspot.com/1963203002/diff/100001/chrome/browser/... chrome/browser/resources/options/content_settings_exceptions_area.js:552: if (item.dataItem.source == 'preference') { no curlies
Updated. Also removed bogus "title" change. PTAL, thanks! https://chromiumcodereview.appspot.com/1963203002/diff/100001/chrome/browser/... File chrome/browser/resources/options/content_settings.css (right): https://chromiumcodereview.appspot.com/1963203002/diff/100001/chrome/browser/... chrome/browser/resources/options/content_settings.css:27: width: 112px; I used Developer Tools, got rid of italic style for Policy for common font, and tweaked value until pixels align. Changed to calc(120px - 8px) hinting at the 120px above. https://chromiumcodereview.appspot.com/1963203002/diff/100001/chrome/browser/... chrome/browser/resources/options/content_settings.css:35: width: 96px; Same thing, though this time aligned to the "new exception" row's <select>. Changing to calc(). https://chromiumcodereview.appspot.com/1963203002/diff/100001/chrome/browser/... chrome/browser/resources/options/content_settings.css:39: text-decoration: line-through; This one is for the <span> that replaces <select>, and the other is for the <div> that replaces <input>. Changing this to .overruled .overruleable {...} and injecting 'overuleable' style to elements that we want to allow line-through, when these elements get instantiated in JS> https://chromiumcodereview.appspot.com/1963203002/diff/100001/chrome/browser/... File chrome/browser/resources/options/content_settings_exceptions_area.js (right): https://chromiumcodereview.appspot.com/1963203002/diff/100001/chrome/browser/... chrome/browser/resources/options/content_settings_exceptions_area.js:243: var indicator = new ControlledSettingIndicator(); I'm moving existing code to this routine for reuse. Yeah the event dance looks odd; just keeping with convension where handlePrefChange() gets notified of changes. https://chromiumcodereview.appspot.com/1963203002/diff/100001/chrome/browser/... chrome/browser/resources/options/content_settings_exceptions_area.js:248: event.value = { controlledBy: controlledBy }; On 2016/05/25 00:41:54, Dan Beam wrote: > no spaces between curlies: > > { controlledBy: controlledBy } > ^ ^ > should be: > > {controlledBy: controlledBy} Done. https://chromiumcodereview.appspot.com/1963203002/diff/100001/chrome/browser/... chrome/browser/resources/options/content_settings_exceptions_area.js:552: if (item.dataItem.source == 'preference') { On 2016/05/25 00:41:53, Dan Beam wrote: > no curlies Done.
Ping dbeam@ for review, PTAL.
i don't like "the event dance", but you are just moving it lgtm
Thanks! Will sync, then commit.
The CQ bit was checked by huangs@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from bauerb@chromium.org, dbeam@chromium.org Link to the patchset: https://chromiumcodereview.appspot.com/1963203002/#ps140001 (title: "Sync.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1963203002/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1963203002/140001
Message was sent while issue was closed.
Description was changed from ========== [Chrome Settings UI] Show ineffective User Exceptions as strike-through. If policy defaults (or other defaults) overrule exceptions, changing UI to show the exceptions texts in strike-through style. - Adding policy icons that display text bubble on click. - Fixing existing text misalignment in "Behavior" column for Policy or Extension Exceptions. This replaces the earlier (buggy) attempt http://crrev.com/1855393006/ BUG=568031 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== [Chrome Settings UI] Show ineffective User Exceptions as strike-through. If policy defaults (or other defaults) overrule exceptions, changing UI to show the exceptions texts in strike-through style. - Adding policy icons that display text bubble on click. - Fixing existing text misalignment in "Behavior" column for Policy or Extension Exceptions. This replaces the earlier (buggy) attempt http://crrev.com/1855393006/ BUG=568031 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ==========
Message was sent while issue was closed.
Committed patchset #8 (id:140001)
Message was sent while issue was closed.
Description was changed from ========== [Chrome Settings UI] Show ineffective User Exceptions as strike-through. If policy defaults (or other defaults) overrule exceptions, changing UI to show the exceptions texts in strike-through style. - Adding policy icons that display text bubble on click. - Fixing existing text misalignment in "Behavior" column for Policy or Extension Exceptions. This replaces the earlier (buggy) attempt http://crrev.com/1855393006/ BUG=568031 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation ========== to ========== [Chrome Settings UI] Show ineffective User Exceptions as strike-through. If policy defaults (or other defaults) overrule exceptions, changing UI to show the exceptions texts in strike-through style. - Adding policy icons that display text bubble on click. - Fixing existing text misalignment in "Behavior" column for Policy or Extension Exceptions. This replaces the earlier (buggy) attempt http://crrev.com/1855393006/ BUG=568031 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/f99222b2b909b00abc34e874a05201de35071737 Cr-Commit-Position: refs/heads/master@{#396914} ==========
Message was sent while issue was closed.
Patchset 8 (id:??) landed as https://crrev.com/f99222b2b909b00abc34e874a05201de35071737 Cr-Commit-Position: refs/heads/master@{#396914} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
