Closed Bug 620258 Opened 9 years ago Closed 9 years ago

add additional fall through comments to nsEventStateManager::PreHandleEvent

Categories

(Core :: DOM: UI Events & Focus Handling, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla5

People

(Reporter: timeless, Assigned: timeless)

References

(Blocks 1 open bug)

Details

(Keywords: coverity)

Attachments

(1 file)

this code already has a couple of fall through comments which helpers readers and static analyzers understand the intent.

adding additional comments in the indicated places will help readers and tools.

1054 nsEventStateManager::PreHandleEvent(nsPresContext* aPresContext,
1055                                     nsEvent *aEvent,
1056                                     nsIFrame* aTargetFrame,
1057                                     nsEventStatus* aStatus,
1058                                     nsIView* aView)

1100   switch (aEvent->message) {
1124   case NS_MOUSE_BUTTON_UP:
1125     switch (static_cast<nsMouseEvent*>(aEvent)->button) {
1126       case nsMouseEvent::eLeftButton:
1127         if (mClickHoldContextMenu) {
1128           KillClickHoldTimer();
1129         }
1130 #ifndef XP_OS2
1131         StopTrackingDragGesture();
1132 #endif
1133         sNormalLMouseEventInProcess = PR_FALSE;

add comment

1134       case nsMouseEvent::eRightButton:
1135 #ifdef XP_OS2
1136         StopTrackingDragGesture();
1137 #endif

add comment

1138       case nsMouseEvent::eMiddleButton:
1139         SetClickCount(aPresContext, (nsMouseEvent*)aEvent, aStatus);
1140         break;
1141     }
1142     break;

1187   case NS_KEY_PRESS:
1188     {
1203       if (modifierMask && (modifierMask == sChromeAccessModifier ||
1204                            modifierMask == sContentAccessModifier))
1205         HandleAccessKey(aPresContext, keyEvent, aStatus, nsnull,
1206                         eAccessKeyProcessingNormal, modifierMask);
1207     }

add comment

1208   case NS_KEY_DOWN:
1209   case NS_KEY_UP:
1210     {
1211       nsIContent* content = GetFocusedContent();
1212       if (content)
1213         mCurrentTargetContent = content;
1214     }
1215     break;
Assignee: nobody → timeless
Status: NEW → ASSIGNED
Attachment #498673 - Flags: review?(Olli.Pettay)
Attachment #498673 - Flags: approval2.0?
Comment on attachment 498673 [details] [diff] [review]
add fallthrough comments

Yeah. This is old code which should be cleaned up.
But better to at least add these comments.
Attachment #498673 - Flags: review?(Olli.Pettay) → review+
Comment on attachment 498673 [details] [diff] [review]
add fallthrough comments

Mass minusing patch approval that don't have high return. Please renominate if this is more important for 2.0 than it appears.
Attachment #498673 - Flags: approval2.0? → approval2.0-
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 589907
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Status: REOPENED → ASSIGNED
http://hg.mozilla.org/mozilla-central/rev/5681a7818bd0
Status: ASSIGNED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.2
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.