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

Issue 716687 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

[MD settings] editing an exception with embeddingOrigin removes the embeddingOrigin

Project Member Reported by dschuyler@chromium.org, Apr 29 2017

Issue description

on chrome://settings/content/images
1. Create an exception with an embeddingOrigin (hack it in).
2. Edit the exception with an embeddingOrigin

When editing an exception with an embedded origin 
   - MD settings removes the embedded origin (changing the exception in a way that cannot be reversed using the MD settings UI).
   - old options adds a new record with the edit (so the old entry is still there)

The old options *only* sends the embedding origin on delete of the item; it is *not* sent on removing for editing.

I/we are not sure that creating a second record is desirable. It may be better to remove the Remove button from the action menu for exceptions that have an embeddingOrigin.
 
A CL to simply not send the embeddingOrigin when editing (matching the behavior of the old options) is at https://codereview.chromium.org/2854523002/
Status: Assigned (was: Untriaged)
Labels: Hotlist-MD-Settings-Privacy-SiteSettings
Cc: msramek@chromium.org benwells@chromium.org raymes@chromium.org
I'm planning to make exceptions that have an embeddingOrigin un-editable. Users will be able to remove them*, but not edit the primary pattern, secondary pattern, or category. The intent is to fix the current, poor experience where editing the exception will clear the embeddingOrigin.

This will leave the future open to benwells@, raymes@, or others who can make a more informed decision about whether the embeddingOrigin should be editable. (Adding the edit feature now may set an unwanted precedent).


*unless they are enforced, but that's normal

Comment 5 by raymes@chromium.org, May 10 2017

That sounds like a good approach to me, thanks! We're currently in the middle of a long process to address the root problem, so having a fix like this sounds appropriate to me.
Status: Started (was: Assigned)
Components: Privacy
Project Member

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

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

commit ad2fac2570c69d70b459d284aa469feab9928dbe
Author: dschuyler <dschuyler@chromium.org>
Date: Wed May 17 02:58:48 2017

[MD settings] content exceptions with embeddingOrigin as read-only

This CL makes content settings exceptions that have an embeddingOrigin
read-only. That means they may be removed (deleted), but they cannot be
edited (not even changing the category).

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

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

[modify] https://crrev.com/ad2fac2570c69d70b459d284aa469feab9928dbe/chrome/browser/resources/settings/site_settings/site_list.html
[modify] https://crrev.com/ad2fac2570c69d70b459d284aa469feab9928dbe/chrome/browser/resources/settings/site_settings/site_list.js
[modify] https://crrev.com/ad2fac2570c69d70b459d284aa469feab9928dbe/chrome/test/data/webui/settings/site_list_tests.js

Status: Fixed (was: Started)

Sign in to add a comment