[PEPC] Implement :granted CSS pseudo class [chromium/src : main]
Thomas Nguyen (Gerrit)
Attention is currently required from: Andy Paicu, Kamila Hasanbega.
Thomas Nguyen would like Andy Paicu and Kamila Hasanbega to review this change.
[PEPC] Implement :granted CSS pseudo class
The pseudo class is used to change the style of <permission> element
when the embedded permission request was explicitly granted from
secondary prompt UI.
Bug:1462930
Change-Id: I8e698cb181a34ce449861031ceea1b8093bf3f23
---
M third_party/blink/renderer/core/css/css_selector.cc
M third_party/blink/renderer/core/css/css_selector.h
M third_party/blink/renderer/core/css/rule_feature_set.cc
M third_party/blink/renderer/core/css/selector_checker.cc
M third_party/blink/renderer/core/html/html_permission_element.cc
M third_party/blink/renderer/core/html/html_permission_element.h
M third_party/blink/renderer/core/html/resources/permission.css
M third_party/blink/renderer/core/inspector/inspector_trace_events.cc
8 files changed, 31 insertions(+), 1 deletion(-)
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Nguyen (Gerrit)
Attention is currently required from: Andy Paicu, Kamila Hasanbega.
Thomas Nguyen (Gerrit)
Attention is currently required from: Andy Paicu, Kamila Hasanbega.
1 comment:
Patchset:
This CL is in the chain of <permission> implementation outlined in https://www.google.com/url?sa=t&rct=j&esrc=s&source=appssearch&uact=8&cd=2&cad=rja&q=&sig2=u2eiOsKQmXXL5jxSAmrFCg&ved=0ahUKEwiD6p_PzZz_AhVdzg0JHSsDCqQ4ABABKAIwAg&url=https://drive.google.com/a/google.com/open%3Fid%3D1a1gjlJ4VkAWoG8AeGKZDcQXm_c0q-cFTs_5MxmjWVYI&usg=AOvVaw3FBSu2yqGjY0A3f8hWlc0y
This CL contains:
- Support new CSS pseudo class :granted, for <permission> element only.
- Default styling is background-color:#ffffff (when permission is granted)
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Nguyen (Gerrit)
Attention is currently required from: Andy Paicu, Kamila Hasanbega, Mason Freed.
Thomas Nguyen would like Mason Freed to review this change.
[PEPC] Implement :granted CSS pseudo class
The pseudo class is used to change the style of <permission> element
when the embedded permission request was explicitly granted from
secondary prompt UI.
Bug:1462930
Change-Id: I8e698cb181a34ce449861031ceea1b8093bf3f23
---
M third_party/blink/renderer/core/css/css_selector.cc
M third_party/blink/renderer/core/css/css_selector.h
M third_party/blink/renderer/core/css/rule_feature_set.cc
M third_party/blink/renderer/core/css/selector_checker.cc
M third_party/blink/renderer/core/html/html_permission_element.cc
M third_party/blink/renderer/core/html/html_permission_element.h
M third_party/blink/renderer/core/html/resources/permission.css
M third_party/blink/renderer/core/inspector/inspector_trace_events.cc
8 files changed, 31 insertions(+), 1 deletion(-)
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Nguyen (Gerrit)
Attention is currently required from: Andy Paicu, Kamila Hasanbega, Mason Freed.
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Andy Paicu (Gerrit)
Attention is currently required from: Kamila Hasanbega, Mason Freed, Thomas Nguyen.
3 comments:
File third_party/blink/renderer/core/html/html_permission_element.h:
Patch Set #2, Line 109:
bool user_granted_permissions_ = false;The granted state should reflect a *truly granted* state where the site will be able to make use of the capability. That includes more things than the user granting permission. Suggestion: `permission_granted_`.
File third_party/blink/renderer/core/html/html_permission_element.cc:
Patch Set #2, Line 193:
user_granted_permissions_ = true;This is likely insufficient. We want to also capture all other permission changes as well as changes back from granted so we'll have to create a subscription to the permission service and listen to permission status changes.
Since we have to do that, I would rather have only one "source of truth" for this information so I would prefer if the state only changed in response to the permission service subscription events.
File third_party/blink/renderer/core/html/resources/permission.css:
Patch Set #2, Line 25:
background-color:#ffffff;We can revisit exact details later. I think for now this is fine as long as we have a task in the spreadsheet to revisit CSS details.
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Nguyen (Gerrit)
Attention is currently required from: Kamila Hasanbega, Thomas Nguyen.
Thomas Nguyen removed Mason Freed from this change.
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Nguyen (Gerrit)
Attention is currently required from: Andy Paicu, Kamila Hasanbega.
3 comments:
File third_party/blink/renderer/core/html/html_permission_element.h:
Patch Set #2, Line 109:
bool user_granted_permissions_ = false;
The granted state should reflect a *truly granted* state where the site will be able to make use of […]
Done
File third_party/blink/renderer/core/html/html_permission_element.cc:
Patch Set #2, Line 193:
user_granted_permissions_ = true;
This is likely insufficient. […]
Oh wait, we expect to update the styling even in case granted by other sources, not only from secondary UI? Let me double check the last discussion to make sure we are not missing any idea.
File third_party/blink/renderer/core/html/resources/permission.css:
Patch Set #2, Line 25:
background-color:#ffffff;
We can revisit exact details later. […]
That's what we agreed at the moment, the background change is the only thing I could find from the current mock. And Acknowledged
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Rune Lillesveen (Gerrit)
Attention is currently required from: Kamila Hasanbega, Thomas Nguyen.
1 comment:
File third_party/blink/renderer/core/html/resources/permission.css:
Patch Set #2, Line 24:
:grantedIs this supposed to be author-exposed? If not it should be ::-internal-*
If it's supposed to be author-exposed it needs a specification. Also, before there is a specification is has to be guarded by a runtime flag.
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Nguyen (Gerrit)
Attention is currently required from: Kamila Hasanbega, Rune Lillesveen.
1 comment:
File third_party/blink/renderer/core/html/resources/permission.css:
Patch Set #2, Line 24:
:granted
Is this supposed to be author-exposed? If not it should be ::-internal-* […]
The <permission> specs are under `Web Incubator` specs and I believe once it's released, we will ask for "granted" specs also.
Currently it's guarded by `PermissionElement` runtime flags, in this CL in
third_party/blink/renderer/core/css/css_selector.cc
third_party/blink/renderer/core/css/selector_checker.ccAm I missing any other places should be guarded?
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Rune Lillesveen (Gerrit)
Attention is currently required from: Kamila Hasanbega, Thomas Nguyen.
1 comment:
File third_party/blink/renderer/core/html/resources/permission.css:
Patch Set #2, Line 24:
:grantedIs this supposed to be author-exposed? If not it should be ::-internal-* […]
Oh. I see it's guarded by a runtime flag.
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Nguyen (Gerrit)
Attention is currently required from: Andy Paicu, Kamila Hasanbega, Rune Lillesveen.
2 comments:
File third_party/blink/renderer/core/html/html_permission_element.cc:
Patch Set #2, Line 193:
user_granted_permissions_ = true;
Oh wait, we expect to update the styling even in case granted by other sources, not only from second […]
We also want to get permission statuses to show a correct string, and I am thinking we will need to expose earlier mojo call: `RegisterPageEmbededPermissionControl` I'd leave a TODO here, given that we should do it in another CL.
File third_party/blink/renderer/core/html/resources/permission.css:
Patch Set #2, Line 24:
:granted
Oh. I see it's guarded by a runtime flag.
Done
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Nguyen (Gerrit)
Attention is currently required from: Andy Paicu, Kamila Hasanbega, Rune Lillesveen.
Thomas Nguyen uploaded patch set #3 to this change.
[PEPC] Implement :granted CSS pseudo class
The pseudo class is used to change the style of <permission> element
when the corresponding permissions are granted.
Bug:1462930
Change-Id: I8e698cb181a34ce449861031ceea1b8093bf3f23
---
M third_party/blink/renderer/core/css/css_selector.cc
M third_party/blink/renderer/core/css/css_selector.h
M third_party/blink/renderer/core/css/rule_feature_set.cc
M third_party/blink/renderer/core/css/selector_checker.cc
M third_party/blink/renderer/core/html/html_permission_element.cc
M third_party/blink/renderer/core/html/html_permission_element.h
M third_party/blink/renderer/core/html/resources/permission.css
M third_party/blink/renderer/core/inspector/inspector_trace_events.cc
8 files changed, 31 insertions(+), 1 deletion(-)
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Nguyen (Gerrit)
Attention is currently required from: Andy Paicu, Kamila Hasanbega, Rune Lillesveen.
Thomas Nguyen uploaded patch set #4 to this change.
[PEPC] Implement :granted CSS pseudo class
The pseudo class is used to change the style of <permission> element
when the corresponding permissions are granted.
Bug:1462930
Change-Id: I8e698cb181a34ce449861031ceea1b8093bf3f23
---
M third_party/blink/public/blink_resources.grd
M third_party/blink/renderer/core/css/css_default_style_sheets.cc
M third_party/blink/renderer/core/css/css_default_style_sheets.h
M third_party/blink/renderer/core/css/css_selector.cc
M third_party/blink/renderer/core/css/css_selector.h
M third_party/blink/renderer/core/css/rule_feature_set.cc
M third_party/blink/renderer/core/css/selector_checker.cc
M third_party/blink/renderer/core/html/html_permission_element.cc
M third_party/blink/renderer/core/html/html_permission_element.h
A third_party/blink/renderer/core/html/resources/permission.css
M third_party/blink/renderer/core/inspector/inspector_trace_events.cc
11 files changed, 111 insertions(+), 1 deletion(-)
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Nguyen (Gerrit)
Attention is currently required from: Andy Paicu, Kamila Hasanbega, Rune Lillesveen.
Thomas Nguyen uploaded patch set #5 to this change.
[PEPC] Implement :granted CSS pseudo class
The pseudo class is used to change the style of <permission> element
when the corresponding permissions are granted.
Bug:1462930
Change-Id: I8e698cb181a34ce449861031ceea1b8093bf3f23
---
M third_party/blink/renderer/core/css/css_selector.cc
M third_party/blink/renderer/core/css/css_selector.h
M third_party/blink/renderer/core/css/rule_feature_set.cc
M third_party/blink/renderer/core/css/selector_checker.cc
M third_party/blink/renderer/core/html/html_permission_element.cc
M third_party/blink/renderer/core/html/html_permission_element.h
M third_party/blink/renderer/core/html/resources/permission.css
M third_party/blink/renderer/core/inspector/inspector_trace_events.cc
8 files changed, 34 insertions(+), 1 deletion(-)
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Nguyen (Gerrit)
Attention is currently required from: Andy Paicu, Kamila Hasanbega, Rune Lillesveen.
Thomas Nguyen would like Rune Lillesveen to review this change.
[PEPC] Implement :granted CSS pseudo class
The pseudo class is used to change the style of <permission> element
when the corresponding permissions are granted.
Bug:1462930
Change-Id: I8e698cb181a34ce449861031ceea1b8093bf3f23
---
M third_party/blink/renderer/core/css/css_selector.cc
M third_party/blink/renderer/core/css/css_selector.h
M third_party/blink/renderer/core/css/rule_feature_set.cc
M third_party/blink/renderer/core/css/selector_checker.cc
M third_party/blink/renderer/core/html/html_permission_element.cc
M third_party/blink/renderer/core/html/html_permission_element.h
M third_party/blink/renderer/core/html/resources/permission.css
M third_party/blink/renderer/core/inspector/inspector_trace_events.cc
8 files changed, 34 insertions(+), 1 deletion(-)
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Nguyen (Gerrit)
Attention is currently required from: Andy Paicu, Kamila Hasanbega, Rune Lillesveen.
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Nguyen (Gerrit)
Attention is currently required from: Andy Paicu, Kamila Hasanbega, Rune Lillesveen.
Thomas Nguyen uploaded patch set #6 to this change.
[PEPC] Implement :granted CSS pseudo class
The pseudo class is used to change the style of <permission> element
when the corresponding permissions are granted.
Bug:1462930
Change-Id: I8e698cb181a34ce449861031ceea1b8093bf3f23
---
M third_party/blink/renderer/core/css/css_selector.cc
M third_party/blink/renderer/core/css/css_selector.h
M third_party/blink/renderer/core/css/rule_feature_set.cc
M third_party/blink/renderer/core/css/selector_checker.cc
M third_party/blink/renderer/core/html/html_permission_element.cc
M third_party/blink/renderer/core/html/html_permission_element.h
M third_party/blink/renderer/core/html/resources/permission.css
M third_party/blink/renderer/core/inspector/inspector_trace_events.cc
8 files changed, 34 insertions(+), 1 deletion(-)
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Nguyen (Gerrit)
Attention is currently required from: Andy Paicu, Kamila Hasanbega, Rune Lillesveen.
Patch Set #2, Line 24:
:granted
Done
@fut...@chromium.org, looks like the css changes are put in your radar automatically, do you mind reviewing the css part?
Thanks
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Nguyen (Gerrit)
Attention is currently required from: Andy Paicu, Kamila Hasanbega, Rune Lillesveen, Thomas Nguyen.
Thomas Nguyen uploaded patch set #8 to this change.
The following approvals got outdated and were removed: Commit-Queue+1 by Thomas Nguyen
[PEPC] Implement :granted CSS pseudo class
The pseudo class is used to change the style of <permission> element
when the corresponding permissions are granted.
Bug:1462930
Change-Id: I8e698cb181a34ce449861031ceea1b8093bf3f23
---
M third_party/blink/renderer/core/css/css_selector.cc
M third_party/blink/renderer/core/css/css_selector.h
M third_party/blink/renderer/core/css/rule_feature_set.cc
M third_party/blink/renderer/core/css/selector_checker.cc
M third_party/blink/renderer/core/html/html_permission_element.cc
M third_party/blink/renderer/core/html/html_permission_element.h
M third_party/blink/renderer/core/html/resources/permission.css
M third_party/blink/renderer/core/inspector/inspector_trace_events.cc
8 files changed, 34 insertions(+), 1 deletion(-)
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Rune Lillesveen (Gerrit)
Attention is currently required from: Andy Paicu, Kamila Hasanbega, Thomas Nguyen.
Patch set 8:Code-Review +1
2 comments:
Patchset:
core/css lgtm
File third_party/blink/renderer/core/html/resources/permission.css:
Patch Set #8, Line 29:
background-color:#ffffff;Nit: inconsistent whether space is used after ':' or not. Seems a space is most common. Also, why not use 'white' instead of the hex value?
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Nguyen (Gerrit)
File third_party/blink/renderer/core/html/resources/permission.css:
Patch Set #8, Line 29:
background-color:#ffffff;
Nit: inconsistent whether space is used after ':' or not. Seems a space is most common. […]
Done
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Thomas Nguyen (Gerrit)
Attention is currently required from: Andy Paicu, Kamila Hasanbega.
Thomas Nguyen uploaded patch set #9 to this change.
[PEPC] Implement :granted CSS pseudo class
The pseudo class is used to change the style of <permission> element
when the corresponding permissions are granted.
Bug:1462930
Change-Id: I8e698cb181a34ce449861031ceea1b8093bf3f23
---
M third_party/blink/renderer/core/css/css_selector.cc
M third_party/blink/renderer/core/css/css_selector.h
M third_party/blink/renderer/core/css/rule_feature_set.cc
M third_party/blink/renderer/core/css/selector_checker.cc
M third_party/blink/renderer/core/html/html_permission_element.cc
M third_party/blink/renderer/core/html/html_permission_element.h
M third_party/blink/renderer/core/html/resources/permission.css
M third_party/blink/renderer/core/inspector/inspector_trace_events.cc
8 files changed, 34 insertions(+), 1 deletion(-)
To view, visit change 4872240. To unsubscribe, or for help writing mail filters, visit settings.
Andy Paicu (Gerrit)
Attention is currently required from: Kamila Hasanbega, Thomas Nguyen.
Patch set 9:Code-Review +1
Thomas Nguyen (Gerrit)
Attention is currently required from: Kamila Hasanbega.
Patch set 9:Commit-Queue +2
Thomas Nguyen (Gerrit)
Attention is currently required from: Kamila Hasanbega.
Patch set 11:Commit-Queue +2
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted this change.
9 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
[PEPC] Implement :granted CSS pseudo class
The pseudo class is used to change the style of <permission> element
when the corresponding permissions are granted.
Bug: 1462930
Change-Id: I8e698cb181a34ce449861031ceea1b8093bf3f23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4872240
Reviewed-by: Rune Lillesveen <fut...@chromium.org>
Reviewed-by: Andy Paicu <andy...@chromium.org>
Commit-Queue: Thomas Nguyen <tun...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1202468}
---
M third_party/blink/renderer/core/css/css_selector.cc
M third_party/blink/renderer/core/css/css_selector.h
M third_party/blink/renderer/core/css/rule_feature_set.cc
M third_party/blink/renderer/core/css/selector_checker.cc
M third_party/blink/renderer/core/html/html_permission_element.cc
M third_party/blink/renderer/core/html/html_permission_element.h
M third_party/blink/renderer/core/html/resources/permission.css
M third_party/blink/renderer/core/inspector/inspector_trace_events.cc
8 files changed, 34 insertions(+), 1 deletion(-)