Closed Bug 393582 Opened 14 years ago Closed 13 years ago
Improve the ns
XULPopup Manager .cpp ns Navigation Direction code
From https://bugzilla.mozilla.org/show_bug.cgi?id=279703#c41 Improve the NavigationDirection code
what kind of improvements are expected here?? On a coarse view, HandleKeyboardNavigationInPopup and HandleKeyboardNavigation code seems to be fine.
The changes described in https://bugzilla.mozilla.org/show_bug.cgi?id=279703#c41
(In reply to comment #3) > Created an attachment (id=296295) [details] > Improve the nsXULPopupManager.cpp nsNavigationDirection code > Doesn't this bring back the warnings that were removed in bug 388356?
Status: NEW → ASSIGNED
OS: Mac OS X → All
Hardware: PC → All
Comment on attachment 296928 [details] [diff] [review] Improve the nsXULPopupManager.cpp nsNavigationDirection code Looks OK, but keep all the 'broken ordering' assertions somewhere, probably where you moved the 'illegal key code' assertion
Attachment #296928 - Flags: review?(enndeakin) → review+
Component: XP Toolkit/Widgets: Menus → XUL
QA Contact: xptoolkit.menus → xptoolkit.widgets
Attachment #354309 - Flags: superreview?(bzbarsky) → superreview+
Comment on attachment 354309 [details] [diff] [review] updated patch >+++ b/layout/xul/base/public/nsXULPopupManager.h >+#if NS_STYLE_DIRECTION_LTR != 0 || NS_STYLE_DIRECTION_RTL != 1 >+ #error "Can't deal with this language" >+#endif Now that we have static asserts, this is probably simpler to write as: PR_STATIC_ASSERT(NS_STYLE_DIRECTION_LTR == 0 && NS_STYLE_DIRECTION_RTL == 1); >-/** >- * DirectionFromKeyCode_rl_tb: an array that maps keycodes to values of >- * nsNavigationDirection for right-to-left and top-to-bottom flow orientation >- * This is defined in nsXULPopupManager.cpp. >- */ Some variant of this comment should likely stay. >+++ b/layout/xul/base/src/nsXULPopupManager.cpp >+ NS_ASSERTION((NS_VK_HOME == NS_VK_END + 1) && >+ (NS_VK_LEFT == NS_VK_END + 2) && >+ (NS_VK_UP == NS_VK_END + 3) && >+ (NS_VK_RIGHT == NS_VK_END + 4) && >+ (NS_VK_DOWN == NS_VK_END + 5), "Broken ordering"); This could also be a PR_STATIC_ASSERT in the header, right? sr=bzbarsky with those nits picked.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.