Issue metadata
Sign in to add a comment
|
a11y: Possible misleading indentation in ax_menu_list_popup.cc |
||||||||||||||||||||||||
Issue description
I was recently doing a Chromium build with GCC, which threw a -Wmisleading-indentation warning when compiling third_party/blink/renderer/modules/accessibility/ax_menu_list_popup.cc:
void AXMenuListPopup::DidHide() {
// ...
if (ActiveDescendant())
cache.PostNotification(this, AXObjectCacheImpl::kAXChildrenChanged);
cache.PostNotification(ActiveDescendant(),
AXObjectCacheImpl::kAXMenuListItemUnselected);
}
The first call to PostNotification() didn't exist until https://chromium-review.googlesource.com/c/chromium/src/+/769674, and now the second one is made even if the if clause doesn't evaluate to true. Since I'm not familiar with the code, I'm filing the bug so that someone else can decide whether the the code just needs braces or if things are working as intended and the code needs to be reindented.
,
Jun 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a628d4ff6707f89c1b8300a0d9e31eb06de0654c commit a628d4ff6707f89c1b8300a0d9e31eb06de0654c Author: DongJun Kim <djmix.kim@samsung.com> Date: Wed Jun 13 08:01:22 2018 Adding missing braces to AXMenuListPopup::DidHide() Chromium build with GCC, it had -Wmisleading-indentation warning in ax_menu_list_popup.cc after commit [1]. It looks unintended and we adds missing braces from previous patch-set. Also this patch includes clean-up as removing redundant function calls. [1] https://chromium-review.googlesource.com/c/chromium/src/+/769674 Bug: 849614 Change-Id: I45051847e116bc055dd8ba954bd9a0730d7188df Reviewed-on: https://chromium-review.googlesource.com/1090698 Commit-Queue: DongJun Kim <djmix.kim@samsung.com> Reviewed-by: Katie Dektar <katie@chromium.org> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org> Cr-Commit-Position: refs/heads/master@{#566755} [modify] https://crrev.com/a628d4ff6707f89c1b8300a0d9e31eb06de0654c/third_party/blink/renderer/modules/accessibility/ax_menu_list_popup.cc
,
Jun 13 2018
Thanks for the patch! |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by djmix....@samsung.com
, Jun 7 2018