Incorporate IsAcceptableSuggestionType into Suggestion::IsAcceptable [chromium/src : main]
0 views
Skip to first unread message
Rahim Mammadli (Gerrit)
4:23 AM (3 hours ago) 4:23 AM
to Jan Keitel, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org
Attention needed from Jan Keitel
Rahim Mammadli voted and added 2 comments![]( "Open in Gerrit")
Votes added by Rahim Mammadli
| Code-Review | +1 |
2 comments
Patchset-level comments
File components/autofill/core/browser/ui/autofill_external_delegate_unittest.cc
Line 2129, Patchset 2 (Latest):
const Suggestion suggestion{SuggestionType::kManageAddress};Rahim Mammadli . unresolved
is this change necessary?
Related details
Attention is currently required from:
- Jan Keitel
Submit Requirements:
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. |
Jan Keitel (Gerrit)
5:11 AM (2 hours ago) 5:11 AM
to Rahim Mammadli, Chromium LUCI CQ, chromium...@chromium.org, browser-comp...@chromium.org
Jan Keitel voted and added 1 comment![]( "Open in Gerrit")
Votes added by Jan Keitel
| Commit-Queue | +2 |
1 comment
File components/autofill/core/browser/ui/autofill_external_delegate_unittest.cc
Line 2129, Patchset 2 (Latest):
const Suggestion suggestion{SuggestionType::kManageAddress};Rahim Mammadli . resolved
Jan Keitelis this change necessary?
Yes, because `AED` CHECKs that `Suggestion::IsAcceptable()` is true.
Related details
Attention set is empty
Submit Requirements:
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)
5:17 AM (2 hours ago) 5:17 AM
to Jan Keitel, Rahim Mammadli, chromium...@chromium.org, browser-comp...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Incorporate IsAcceptableSuggestionType into Suggestion::IsAcceptable
This CL extends autofill::Suggestion::IsAcceptable() to check whether the
suggestion type is acceptable (e.g. not a separator, title, or message)
in addition to checking acceptability state.
The standalone helper IsAcceptableSuggestionType() is removed and its
call sites updated to call Suggestion::IsAcceptable() instead.
While still not perfect (we should consider adding setters and getters
to Suggestion as it becomes more complex), this makes acceptability
decisions more centralized and, hopefully, less confusing.
TAG=agy
CONV=8d0cc70f-64a0-4a23-a918-b82a2404358c
Bug: 40100455, 537193970
Change-Id: I47683d6e8ccdb0650679907162e2b95640f09d82
Reviewed-by: Rahim Mammadli <rah...@google.com>
Commit-Queue: Jan Keitel <jke...@google.com>
Cr-Commit-Position: refs/heads/main@{#1667732}
Files:
- M chrome/browser/ui/autofill/autofill_keyboard_accessory_controller_impl.cc
- M chrome/browser/ui/autofill/autofill_popup_controller_impl.cc
- M chrome/browser/ui/autofill/autofill_suggestion_controller_utils.cc
- M chrome/browser/ui/autofill/autofill_suggestion_controller_utils.h
- M components/autofill/core/browser/BUILD.gn
- M components/autofill/core/browser/suggestions/suggestion.cc
- M components/autofill/core/browser/suggestions/suggestion.h
- A components/autofill/core/browser/suggestions/suggestion_unittest.cc
- M components/autofill/core/browser/ui/autofill_external_delegate_unittest.cc
Change size: M
Delta: 9 files changed, 58 insertions(+), 19 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Rahim Mammadli
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages