Closed Bug 1085628 Opened 7 years ago Closed 7 years ago
_880164 _customization _context _menus .js fails in OSX when the toolbar size is smaller than default
The following test fails to execute against Firefox dev edition, with a custom theme: browser/components/customizableui/test/browser_880164_customization_context_menus.js The fake right click dispatched over here: http://mxr.mozilla.org/mozilla-central/source/browser/components/customizableui/test/browser_880164_customization_context_menus.js#119 is being done at a wrong position. I don't know exactly why, the (100, urlbarRect.height - 1) doesn't work on FF dev ed. The size of the toolbar is really smaller and there might be some padding/margin differences around the UI element being tested. Clicking on (100, 1) works fine... https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=f943af7cb111
See also: https://bugzilla.mozilla.org/show_bug.cgi?id=1053434#c101. Gijs, is clicking on (100, 1) instead of (100, urlbarRect.height-1) a reasonable fix for this?
Comment on attachment 8508220 [details] [diff] [review] patch Review of attachment 8508220 [details] [diff] [review]: ----------------------------------------------------------------- Assuming tests pass on fx-team/try without further changes, it's an OK fix. As I said elsewhere, the proper fix might be adding a separate item with removable="false" set. If we don't need to do that here/yet, we can postpone that to get stuff in working order for devedition.
Attachment #8508220 - Flags: review+
(can someone fix the bug and patch summary, though?)
Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=81387bbd4b28. Here is a push without the patch: https://tbpl.mozilla.org/?tree=Try&rev=b48499fdc04e
The try runs are going as expected (orange without the patch and green with it)
Summary: Strengthens customization context menu test → browser_880164_customization_context_menus.js fails in OSX when the toolbar size is smaller than default
New commit message, with a try run with the patch on top of m-c instead of gum: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=5bcde157f4a6
Attachment #8508220 - Attachment is obsolete: true
Assignee: nobody → poirot.alex
Status: NEW → ASSIGNED
Comment on attachment 8508562 [details] [diff] [review] patch Carrying over r+
Attachment #8508562 - Flags: review+
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 36
Comment on attachment 8508562 [details] [diff] [review] patch Approval Request Comment [Feature/regressing bug #]: it's a test improvement that will be required once we uplift some other patches that will make the failure permanent [User impact if declined]: none, only test failures [Describe test coverage new/current, TBPL]: existing test [Risks and why]: none, it's a test-only change [String/UUID change made/needed]: none
Comment on attachment 8508562 [details] [diff] [review] patch Apparently test-only fixes don't need approval.
You need to log in before you can comment on or make changes to this bug.