Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1704333002:
Do not use Node in FocusController (Closed)
DescriptionDo not use Node in FocusController
Since a focus navigation operates on only elements, FocusController should use Element, instead of Node,
as much as possible, so it becomes more type safe and avoid an unnecessary `toElement` type cast.
ShadowRoot is one of the exceptions so far. This CL can avoid passing a ShadowRoot by introducing findFocusableElementInShadowHost().
FocusController::findFocusableElement() no longer needs to be a public member function, making it private.
This is the first wave to replace Node with Element in FocusController. I'll replace other parts in another CL.
BUG=587743
Committed: https://crrev.com/818f579b3cd4f05e55b55d88ac899694d945d574
Cr-Commit-Position: refs/heads/master@{#376379}
Patch Set 1 #Patch Set 2 : updated #Patch Set 3 : split a CL #
Dependent Patchsets: Messages
Total messages: 29 (17 generated)
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1704333002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1704333002/1
hayato@chromium.org changed reviewers: + kochi@chromium.org, yuzus@chromium.org
PTAL
Description was changed from ========== Replace Node with Element in FocusController. This is the first wave to replace Node with Element in FocusController. I'll replace other parts in another CL. BUG=587743 ========== to ========== Replace Node with Element in FocusController This is the first attempt to replace Node with Element in FocusController. I'll replace other parts in another CL. BUG=587743 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
updated
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1704333002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1704333002/20001
Description was changed from ========== Replace Node with Element in FocusController This is the first attempt to replace Node with Element in FocusController. I'll replace other parts in another CL. BUG=587743 ========== to ========== Do not use Node in FocusController Since a focus navigation operates on only elements, we should use Element as type as much as possible in FocusController. This is the first wave to replace Node with Element in FocusController. I'll replace other parts in another CL. BUG=587743 ==========
Description was changed from ========== Do not use Node in FocusController Since a focus navigation operates on only elements, we should use Element as type as much as possible in FocusController. This is the first wave to replace Node with Element in FocusController. I'll replace other parts in another CL. BUG=587743 ========== to ========== Do not use Node in FocusController Since a focus navigation operates on only elements, we should use Element, instead of Node, as much as possible in FocusController. This is the first wave to replace Node with Element in FocusController. I'll replace other parts in another CL. BUG=587743 ==========
Description was changed from ========== Do not use Node in FocusController Since a focus navigation operates on only elements, we should use Element, instead of Node, as much as possible in FocusController. This is the first wave to replace Node with Element in FocusController. I'll replace other parts in another CL. BUG=587743 ========== to ========== Do not use Node in FocusController Since a focus navigation operates on only elements, we should use Element, instead of Node, as much as possible in FocusController. ShadowRoot is an exception so far. This CL can avoid to handle ShadowRoot by introducing findFocusableElementInShadowHost(*this). FocusController::findFocusableElement() is no longer public function. Thus make it a private member function. This is the first wave to replace Node with Element in FocusController. I'll replace other parts in another CL. BUG=587743 ==========
Description was changed from ========== Do not use Node in FocusController Since a focus navigation operates on only elements, we should use Element, instead of Node, as much as possible in FocusController. ShadowRoot is an exception so far. This CL can avoid to handle ShadowRoot by introducing findFocusableElementInShadowHost(*this). FocusController::findFocusableElement() is no longer public function. Thus make it a private member function. This is the first wave to replace Node with Element in FocusController. I'll replace other parts in another CL. BUG=587743 ========== to ========== Do not use Node in FocusController Since a focus navigation operates on only elements, we should use Element, instead of Node, as much as possible in FocusController. ShadowRoot is an exception so far. This CL can avoid to pass a ShadowRoot by introducing findFocusableElementInShadowHost(*this). FocusController::findFocusableElement() no longer needs to be a public member function, making it private. This is the first wave to replace Node with Element in FocusController. I'll replace other parts in another CL. BUG=587743 ==========
Description was changed from ========== Do not use Node in FocusController Since a focus navigation operates on only elements, we should use Element, instead of Node, as much as possible in FocusController. ShadowRoot is an exception so far. This CL can avoid to pass a ShadowRoot by introducing findFocusableElementInShadowHost(*this). FocusController::findFocusableElement() no longer needs to be a public member function, making it private. This is the first wave to replace Node with Element in FocusController. I'll replace other parts in another CL. BUG=587743 ========== to ========== Do not use Node in FocusController Since a focus navigation operates on only elements, FocusController should use Element, instead of Node, as much as possible to be type safe and avoid unnecessary toElement cast. ShadowRoot is an exception so far. This CL can avoid to pass a ShadowRoot by introducing findFocusableElementInShadowHost(*this). FocusController::findFocusableElement() no longer needs to be a public member function, making it private. This is the first wave to replace Node with Element in FocusController. I'll replace other parts in another CL. BUG=587743 ==========
Description was changed from ========== Do not use Node in FocusController Since a focus navigation operates on only elements, FocusController should use Element, instead of Node, as much as possible to be type safe and avoid unnecessary toElement cast. ShadowRoot is an exception so far. This CL can avoid to pass a ShadowRoot by introducing findFocusableElementInShadowHost(*this). FocusController::findFocusableElement() no longer needs to be a public member function, making it private. This is the first wave to replace Node with Element in FocusController. I'll replace other parts in another CL. BUG=587743 ========== to ========== Do not use Node in FocusController Since a focus navigation operates on only elements, FocusController should use Element, instead of Node, as much as possible, so it is more type safe and avoid unnecessary toElement type cast. ShadowRoot is an exception so far. This CL can avoid to pass a ShadowRoot by introducing findFocusableElementInShadowHost(*this). FocusController::findFocusableElement() no longer needs to be a public member function, making it private. This is the first wave to replace Node with Element in FocusController. I'll replace other parts in another CL. BUG=587743 ==========
Description was changed from ========== Do not use Node in FocusController Since a focus navigation operates on only elements, FocusController should use Element, instead of Node, as much as possible, so it is more type safe and avoid unnecessary toElement type cast. ShadowRoot is an exception so far. This CL can avoid to pass a ShadowRoot by introducing findFocusableElementInShadowHost(*this). FocusController::findFocusableElement() no longer needs to be a public member function, making it private. This is the first wave to replace Node with Element in FocusController. I'll replace other parts in another CL. BUG=587743 ========== to ========== Do not use Node in FocusController Since a focus navigation operates on only elements, FocusController should use Element, instead of Node, as much as possible, so it becomes more type safe and avoid an unnecessary `toElement` type cast. ShadowRoot is one of the exceptions so far. This CL can avoid passing a ShadowRoot by introducing findFocusableElementInShadowHost(). FocusController::findFocusableElement() no longer needs to be a public member function, making it private. This is the first wave to replace Node with Element in FocusController. I'll replace other parts in another CL. BUG=587743 ==========
The CQ bit was checked by hayato@chromium.org to run a CQ dry run
split a CL
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1704333002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1704333002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by hayato@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1704333002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1704333002/40001
Message was sent while issue was closed.
Description was changed from ========== Do not use Node in FocusController Since a focus navigation operates on only elements, FocusController should use Element, instead of Node, as much as possible, so it becomes more type safe and avoid an unnecessary `toElement` type cast. ShadowRoot is one of the exceptions so far. This CL can avoid passing a ShadowRoot by introducing findFocusableElementInShadowHost(). FocusController::findFocusableElement() no longer needs to be a public member function, making it private. This is the first wave to replace Node with Element in FocusController. I'll replace other parts in another CL. BUG=587743 ========== to ========== Do not use Node in FocusController Since a focus navigation operates on only elements, FocusController should use Element, instead of Node, as much as possible, so it becomes more type safe and avoid an unnecessary `toElement` type cast. ShadowRoot is one of the exceptions so far. This CL can avoid passing a ShadowRoot by introducing findFocusableElementInShadowHost(). FocusController::findFocusableElement() no longer needs to be a public member function, making it private. This is the first wave to replace Node with Element in FocusController. I'll replace other parts in another CL. BUG=587743 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Do not use Node in FocusController Since a focus navigation operates on only elements, FocusController should use Element, instead of Node, as much as possible, so it becomes more type safe and avoid an unnecessary `toElement` type cast. ShadowRoot is one of the exceptions so far. This CL can avoid passing a ShadowRoot by introducing findFocusableElementInShadowHost(). FocusController::findFocusableElement() no longer needs to be a public member function, making it private. This is the first wave to replace Node with Element in FocusController. I'll replace other parts in another CL. BUG=587743 ========== to ========== Do not use Node in FocusController Since a focus navigation operates on only elements, FocusController should use Element, instead of Node, as much as possible, so it becomes more type safe and avoid an unnecessary `toElement` type cast. ShadowRoot is one of the exceptions so far. This CL can avoid passing a ShadowRoot by introducing findFocusableElementInShadowHost(). FocusController::findFocusableElement() no longer needs to be a public member function, making it private. This is the first wave to replace Node with Element in FocusController. I'll replace other parts in another CL. BUG=587743 Committed: https://crrev.com/818f579b3cd4f05e55b55d88ac899694d945d574 Cr-Commit-Position: refs/heads/master@{#376379} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/818f579b3cd4f05e55b55d88ac899694d945d574 Cr-Commit-Position: refs/heads/master@{#376379} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
