[PEPC] Refactor request logic to fix click-to-re-grant regression [chromium/src : main]
Thomas Nguyen (Gerrit)
Thomas Nguyen added 1 comment![]( "Open in Gerrit")
Hi Joey, could you please take a look at this?
This change fixes a regression bug, and I added a new test for it. Additionally, I tidied up the code a bit, got rid of some duplicate, and added that clearWatch correctly.
Related details
- Joey Arhar
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
AI Code Reviewer (Gerrit)
AI Code Reviewer added 1 comment![]( "Open in Gerrit")
void MaybeTriggerAutolocate(ForceAutolocate force);Blink Style Guide: Naming - May leave obvious parameter names out of function declarations. Obvious parameter names should be omitted from declarations in header files when the type name makes the purpose clear. Please change this to `void MaybeTriggerAutolocate(ForceAutolocate);`.
_To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **Won't fix**: reason | **b/<bug_id>** | **Invalid:** reason
This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent).AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve.[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
Related details
- Joey Arhar
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thomas Nguyen (Gerrit)
Thomas Nguyen added 1 comment![]( "Open in Gerrit")
void MaybeTriggerAutolocate(ForceAutolocate force);Blink Style Guide: Naming - May leave obvious parameter names out of function declarations. Obvious parameter names should be omitted from declarations in header files when the type name makes the purpose clear. Please change this to `void MaybeTriggerAutolocate(ForceAutolocate);`.
_To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **Won't fix**: reason | **b/<bug_id>** | **Invalid:** reasonThis comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent).AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve.[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
Done
Related details
- Joey Arhar
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joey Arhar (Gerrit)
Joey Arhar voted Code-Review+1![]( "Open in Gerrit")
- Thomas Nguyen
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thomas Nguyen (Gerrit)
Thomas Nguyen voted Commit-Queue+2![]( "Open in Gerrit")
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thomas Nguyen (Gerrit)
Thomas Nguyen voted Commit-Queue+0![]( "Open in Gerrit")
| Commit-Queue | +0 |
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thomas Nguyen (Gerrit)
Thomas Nguyen voted and added 1 comment![]( "Open in Gerrit")
Votes added by Thomas Nguyen
| Commit-Queue | +2 |
1 comment
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thomas Nguyen (Gerrit)
Thomas Nguyen voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
5 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: third_party/blink/renderer/core/html/html_geolocation_element.cc
Insertions: 3, Deletions: 1.
@@ -150,7 +150,9 @@
}
if (FastHasAttribute(html_names::kAutolocateAttr)) {
- MaybeTriggerAutolocate(ForceAutolocate::kNo);
+ MaybeTriggerAutolocate(HasPendingPermissionRequest()
+ ? ForceAutolocate::kYes
+ : ForceAutolocate::kNo);
} else if (HasPendingPermissionRequest()) {
RequestGeolocation();
}
```
Change information
[PEPC] Refactor request logic to fix click-to-re-grant regression
A previous change introduced a regression where clicking a
<geolocation>` element without the `autolocate` attribute would fail to
trigger a location request.
This CL refactors the location request logic to fix this regression
and correctly handle all user interaction scenarios.
- M third_party/blink/renderer/core/html/html_geolocation_element.cc
- M third_party/blink/renderer/core/html/html_geolocation_element.h
- M third_party/blink/renderer/core/html/html_geolocation_element_test.cc
- M third_party/blink/renderer/core/html/html_permission_element.h
Code-Review: +1 by Joey Arhar
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |