[TabGroups] Add support for closing tab groups via middle mouse click [chromium/src : main]
Jay Kapadia (Gerrit)
Jay Kapadia added 1 comment![]( "Open in Gerrit")
Hello mentors,
I have implemented the functionality and added the corresponding test. The functionality is working perfectly. However, I am unable to run the test individually.
Could you please suggest the correct command to run it?
Related details
- Alison Gale
- David Pennington
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alison Gale (Gerrit)
Alison Gale added 2 comments![]( "Open in Gerrit")
Hello mentors,
I have implemented the functionality and added the corresponding test. The functionality is working perfectly. However, I am unable to run the test individually.
Could you please suggest the correct command to run it?
To run an interactive UI test, you would build it with the command `autoninja -C out\Default interactive_ui_tests` and then you can run a single test with the command `out\Default\interactive_ui_tests --gtest_filter=SavedTabGroupInteractiveTest.CloseTabGroupOnMiddleMouseClick`
- David Pennington
- Jay Kapadia
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jay Kapadia (Gerrit)
Jay Kapadia added 2 comments![]( "Open in Gerrit")
Alison GaleHello mentors,
I have implemented the functionality and added the corresponding test. The functionality is working perfectly. However, I am unable to run the test individually.
Could you please suggest the correct command to run it?
To run an interactive UI test, you would build it with the command `autoninja -C out\Default interactive_ui_tests` and then you can run a single test with the command `out\Default\interactive_ui_tests --gtest_filter=SavedTabGroupInteractiveTest.CloseTabGroupOnMiddleMouseClick`
Thank you, it worked.
I corrected some mistakes in the test.
Also, while running the test I was getting EXCEPTION_ACCESS_VIOLATION on `tab_slot_controller_->EndDrag(END_DRAG_COMPLETE);`. So, I returned early from the if block. According to me this should not cause any side effects but I request that you verify it once.
Related details
- Alison Gale
- David Pennington
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alison Gale (Gerrit)
Alison Gale added 1 comment![]( "Open in Gerrit")
I corrected some mistakes in the test.
Also, while running the test I was getting EXCEPTION_ACCESS_VIOLATION on `tab_slot_controller_->EndDrag(END_DRAG_COMPLETE);`. So, I returned early from the if block. According to me this should not cause any side effects but I request that you verify it once.
Does it work if the drag is ended before the tab group is closed? I was curious so I looked at the corresponding implementation in tab.cc and that appears to be what they are doing. My guess is that if it tries to end a drag after tabs have already been deleted it would cause a problem.
Related details
- David Pennington
- Jay Kapadia
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jay Kapadia (Gerrit)
Jay Kapadia added 1 comment![]( "Open in Gerrit")
I ended the drag before the if block. This way END_DRAG is called unconditionally and it is called before the tab group potentially closes.
I tried it, it is working and test is also working as expected.
Related details
- Alison Gale
- David Pennington
Code-Coverage
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alison Gale (Gerrit)
Alison Gale added 1 comment![]( "Open in Gerrit")
tab_slot_controller_->EndDrag(END_DRAG_COMPLETE);
if (!dragging()) {David, do you have context on whether we would need to cache the !dragging value before ending the drag above or are those unrelated variables? This is a similar pattern to what is used in tab.cc. Could just throw it behind a flag if we want to be extra safe when landing
Related details
- David Pennington
- Jay Kapadia
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jay Kapadia (Gerrit)
Jay Kapadia added 1 comment![]( "Open in Gerrit")
The test is failing on `mac-rel` run, likely because the middle mouse click is not being simulated correctly on mac devices. On doing some research I found that mac does not support middle clicks.
Should I exclude mac from the test?
Related details
Code-Coverage
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |