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

Issue 662919 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug

Blocking:
issue 614588



Sign in to add a comment

Pattern with wildcards shown incorrectly in content-settings

Project Member Reported by dullweber@chromium.org, Nov 7 2016

Issue description

When 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?
 
Cc: finnur@chromium.org dschuyler@chromium.org
The issue seems to be toUrl() from here: https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/site_settings/site_settings_behavior.js?l=418

Why is this done? It hides information that is needed to understand exceptions based on pattern.

Comment 2 by rolfe@chromium.org, Nov 8 2016

Cc: tbuck...@chromium.org
+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)
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.

#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).



Status: Assigned (was: Untriaged)
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. 
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?
Components: Privacy
Labels: Team-Security-UX
> Shall we start a design doc with these considerations?

I think that makes sense.
Cc: dbeam@chromium.org
>> Shall we start a design doc with these considerations?

> I think that makes sense.

SGTM.

Comment 11 by rolfe@chromium.org, Nov 11 2016

Cc: lgar...@chromium.org emilyschechter@chromium.org est...@chromium.org
+ some interested parties
Blocking: 614588
Labels: Proj-MaterialDesign-WebUI
Owner: dschuyler@chromium.org
Status: Started (was: Assigned)
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).

6X71MiG.png
15.4 KB View Download

Comment 13 by rolfe@chromium.org, Nov 11 2016

LGTM, but I defer to bettes@.
The screen shot in #12 is showing what it will change to. Where the different exceptions are not combined and the [*.] patterns are shown.
Project Member

Comment 15 by bugdroid1@chromium.org, 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

Status: Fixed (was: Started)
Status: Verified (was: Fixed)

Sign in to add a comment