Pattern with wildcards shown incorrectly in content-settings |
|||||||||
Issue descriptionWhen adding a pattern in content-setting with a wildcard like "[*.]google.com" it is shown as "http://google.com". (e.g. in chrome://md-settings/content/location) The old settings menu shows it as [*.]google.com so it is probably added correctly. Ports are also not shown. I would expect pattern to be shown as stored. Is that a bug or intentional behavior?
,
Nov 8 2016
+tbuckley as FYI Doesn't sound like UX can help much here, as yes ideally the old pattern would persist with the revamped design (unless there's some context I'm not aware of. Curious to the answer to Comment No. 1)
,
Nov 8 2016
Thanks rolfe@! If this wasn't a UI decision, finnur@ and dschuyler@, do you have the background behind it? The current behavior shows http-and-https exceptions as http-only, which seems dangerous. Furthermore, the deduplication of exceptions that aren't really duplicate causes a weird behavior where you click "Remove" on an exception but it stays there. The purpose of this UI is to give the user insight into permissions they have granted, so I don't think that obscuring things is a good idea.
,
Nov 8 2016
#3 finnur@ would have more insight on why removal of the patterns was initially done. I'll try to answer why I've kept that after Finnur handed off this area of the code to me. First I'd like to point out a related topic The rules are further obscured by grouping them by Allow, Session, and Block. If we'd like to allow users to see and reason about how the rules are applied, both of these UI changes would need to be reverted back to something closer to the old UI. bettes@ and tbuckley@ are we up for changing how these are displayed? I know there have been previous discussions about the grouping of the exceptions. IMO the conversations about grouping the site exceptions and showing patterns should considered together. Overall, I think we need a firm call on whether editing site exceptions is a power user feature or whether it should not be a power user feature. The other decisions fall out of that decision. - If site exceptions are a power user feature intended to relay how exception rules are applied then we should un-group the exceptions and show the patterns. - If the site exceptions are being friendlified for non-power users then it makes sense to group the exceptions and clean off the encodings for the patterns. - Using a mix of those directions will make a feature that isn't accurate/useful for a power user and still weird/cryptic for a non-power user. There is value in the power user UI and in the non-power user UI. I'd simply like to avoid landing on a confused mix of those directions. It may be worthwhile to provide one of them (maybe the power user UI) as a separate UI through an alternate UI (such as an extension). If the ability to review and reason about the site exceptions is considered a feature of the current options, we would need show the patterns and the order the rules are applied (un-group) or we would need to be more explicit that this feature is being intentionally removed (or being moved elsewhere).
,
Nov 8 2016
,
Nov 8 2016
It's not that patterns have been removed -- the underlying API still uses patterns. This is a question of how they are presented in the UI. It was my understanding that one of the intents of Site Settings was to make the UI more accessible to regular people. The old UI seems written by engineers for engineers, whereas the new UI is a bit more friendly. To accomplish this, UX designed a site-centric UI with the mocks showing URLS, not patterns. And if we are to map patterns into that world then you need some normalization to occur. For example, it doesn't make sense for permissions for http://[*.]google.com and http://[*.]google.com:80 to be under two sites, for example. Makes much more sense to group them together. Some of these normalizations are probably non-controversial (think http:80 and https:443). Some of these are harder (complex custom exceptions, perhaps). And then, by the sound of it, there are some we're getting wrong: I guess [*.]google.com should show up as two sites in the UI: one as http://google.com and one as https://google.com. It would be easier, though, implementation-wise if it could show up under one site (just google.com) and the permission clearly stating that it applies to both http and https. I also don't think people think of google.com and google.com:123 as different sites, so it makes sense to group those together, but we should still show in the UI when a permission applies only to a single port. How to do those two things properly is a question for UX, I suppose. There might also be some trickier aspect to more complex patterns that I haven't thought about. The goal, however, was not solely to convert the old UI to material design, so we should expect less emphasis on showing patterns than in the old UI.
,
Nov 9 2016
Thanks for the explanations! IIRC the last discussion (that I was part of?) ended up with (long-term, probably not immediate) solution: a) The "All sites" menu which shows sites, and every site subpage lists the exceptions that apply. For example, we show the site "google.com", you click on it, and then we can show you that it's affected by exceptions like "http://[*.]google.com" etc. b) The per-content-type subpages should just contain patterns (split by allow/block values). So if I click "Geolocation", I should see all patterns for geolocation, and not sites. Which is a great solution, because we have both the simple and the power-user interace. But regardless of what the solution exactly is, no simplification we make should *ever* hide an exception or make it impossible to distinguish between two exceptions. I think we should start from the fact that nowadays most exceptions are origin-based, so those should look nicest. For example, as finnur@ mentioned, we can simplify ports. The current state is: https://google.com:443 <-- port 443 https://google.com <-- all ports We can simplify this to https://google.com <-- port 443 https://google.com:* <-- all ports because "all ports" exceptions are typically added by power users, so they can be "uglier". However, I don't think we can simplify "[*.]". Users will mostly encounter the origin-scoped "google.com", which is fine. Occasionally, they will see the broader domain-scoped "[*.]google.com", and in that case, it's also important to call out that it's something different than usual. If it doesn't look good, we can spell out "google.com and subdomains". Even if a user doesn't understand what a subdomain is, it's still easier to google the word "subdomain" than the "[*.]" syntax. ...etc., etc. Shall we start a design doc with these considerations?
,
Nov 9 2016
,
Nov 9 2016
> Shall we start a design doc with these considerations? I think that makes sense.
,
Nov 9 2016
>> Shall we start a design doc with these considerations? > I think that makes sense. SGTM.
,
Nov 11 2016
+ some interested parties
,
Nov 11 2016
I'd like this conversation to continue. It's important that we have clear UI. In the near term, let's be more explicit with the exceptions (i.e. not change them from the options UI).
,
Nov 11 2016
LGTM, but I defer to bettes@.
,
Nov 11 2016
The screen shot in #12 is showing what it will change to. Where the different exceptions are not combined and the [*.] patterns are shown.
,
Nov 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c03ac1f988225abce504a023f29db90c6dbb2872 commit c03ac1f988225abce504a023f29db90c6dbb2872 Author: dschuyler <dschuyler@chromium.org> Date: Sat Nov 12 01:54:44 2016 [MD settings] show full origin in content settings exceptions This CL removes the filtering from the content settings excptions to show the full exception rule. BUG= 662919 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2494943003 Cr-Commit-Position: refs/heads/master@{#431736} [modify] https://crrev.com/c03ac1f988225abce504a023f29db90c6dbb2872/chrome/browser/resources/settings/site_settings/site_settings_behavior.js [modify] https://crrev.com/c03ac1f988225abce504a023f29db90c6dbb2872/chrome/test/data/webui/settings/site_list_tests.js
,
Nov 12 2016
,
Apr 5 2017
|
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by dullweber@chromium.org
, Nov 7 2016