Code-Review | -1 |
Reminder to self: don't land until devrel is done with demos
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Code-Review | +0 |
Hopefully the latest patchset passes, but I want to get a start on the review for this patch, because it's large. LMK what you see.
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Code-Review | +1 |
"interestaction",
interestaction looks like its an attribute in svg_attribute_names. should we leave it in here?
// have details relationships with their target, when that interest for is
Is this sentence complete?
// Interest for invoking elements (with the `interestfor` attribute) should
// have details relationships with their target, when that interest for is
// a) visible, b) is rich, and c) not the next element in the DOM (depth first
// search order).
Should there be a comment for this method both in the header and the cc file? I'd consider only commenting one of the two.
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
"interestaction",
interestaction looks like its an attribute in svg_attribute_names. should we leave it in here?
Nope - removed.
// have details relationships with their target, when that interest for is
Mason FreedIs this sentence complete?
Weird. I have no idea where that came from, but the whole new chunk is now gone.
// Interest for invoking elements (with the `interestfor` attribute) should
// have details relationships with their target, when that interest for is
// a) visible, b) is rich, and c) not the next element in the DOM (depth first
// search order).
Should there be a comment for this method both in the header and the cc file? I'd consider only commenting one of the two.
Well, I guess the purpose and content are different. I always put the "how to use this" help in the .h file, since that's what an IDE will show you as you autocomplete it. And then if needed, I put this kind of comment here, which is "how does this actually work" - about how it's implemented. That matter when someone else comes to change it later.
Ok to keep both?
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Code-Review | +1 |
// Interest for invoking elements (with the `interestfor` attribute) should
// have details relationships with their target, when that interest for is
// a) visible, b) is rich, and c) not the next element in the DOM (depth first
// search order).
Mason FreedShould there be a comment for this method both in the header and the cc file? I'd consider only commenting one of the two.
Well, I guess the purpose and content are different. I always put the "how to use this" help in the .h file, since that's what an IDE will show you as you autocomplete it. And then if needed, I put this kind of comment here, which is "how does this actually work" - about how it's implemented. That matter when someone else comes to change it later.
Ok to keep both?
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Auto-Submit | +0 |
Now, of course, I need lots of reviewers. Thanks in advance!
twellington:
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulator.java
chrome/android/java/src/org/chromium/chrome/browser/contextmenu/ContextMenuCoordinator.java
chrome/android/javatests/src/org/chromium/chrome/browser/contextmenu/ChromeContextMenuPopulatorTest.java
chrome/android/javatests/src/org/chromium/chrome/browser/contextmenu/ContextMenuTest.java
chrome/android/junit/src/org/chromium/chrome/browser/contextmenu/ContextMenuCoordinatorTest.java
chrome/android/junit/src/org/chromium/chrome/browser/contextmenu/ContextMenuHeaderMediatorTest.java
chrome/browser/resources/chromeos/accessibility/definitions/automation.d.ts
dcheng:
third_party/blink/common/context_menu_data/context_menu_mojom_traits.cc
third_party/blink/common/context_menu_data/context_menu_params_builder.cc
third_party/blink/common/context_menu_data/untrustworthy_context_menu_params.cc
third_party/blink/public/common/context_menu_data/context_menu_data.h
third_party/blink/public/common/context_menu_data/context_menu_mojom_traits.h
third_party/blink/public/common/context_menu_data/untrustworthy_context_menu_params.h
third_party/blink/public/devtools_protocol/browser_protocol.pdl
third_party/blink/public/mojom/context_menu/context_menu.mojom
third_party/blink/public/mojom/frame/frame.mojom
third_party/blink/public/strings/blink_strings.grd
ui/accessibility/ax_enums.mojom
boliu:
android_webview/javatests/src/org/chromium/android_webview/test/ContextMenuTest.java
components/embedder_support/android/contextmenu/context_menu_builder.cc
components/embedder_support/android/java/src/org/chromium/components/embedder_support/contextmenu/ContextMenuParams.java
components/embedder_support/android/junit/src/org/chromium/components/embedder_support/contextmenu/ContextMenuUtilsUnitTest.java
content/browser/renderer_host/page_impl.h
dtseng:
chrome/browser/resources/chromeos/accessibility/definitions/automation.d.ts
content/browser/accessibility/dump_accessibility_tree_browsertest.cc
content/browser/accessibility/dump_accessibility_tree_browsertest.h
extensions/common/api/automation.idl
ui/accessibility/ax_computed_node_data.cc
ui/accessibility/ax_enum_util.cc
ui/accessibility/platform/ax_platform_node_base.cc
ui/accessibility/platform/inspect/ax_tree_formatter_auralinux.cc
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Code-Review | +1 |
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Commit-Queue | +2 |
Thanks for the quick +1's everyone. :-)
Rename interesttarget attribute and friends
**** GARDENERS: Please note! ****
Please do me a huge favor and don't revert this CL if there are a
few test flakes after landing it. Instead, please just mark those
tests as flaky/fail and assign me a bug and I'll get it cleaned up.
Thanks in advance!
*********************************
There have been a few standards conversations about naming:
- https://github.com/w3c/csswg-drafts/issues/9236#issuecomment-2940736593
- https://github.com/whatwg/html/issues/11312
Per those conversations, this CL renames the following:
- the `interesttarget` attribute becomes the `interestfor` attribute
- `interestTargetElement` -> `interestForElement`
- CSS `interest-target-*-delay` -> `interest-*-delay`
I also renamed the feature flags and associated code, so that the new
names are code-searchable. I did not rename the IDS string var names,
so that we don't waste time on re-translating them.
Bug: 364669918
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/53356
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |