Closed
Bug 1387182
Opened 7 years ago
Closed 7 years ago
Toolbar icons "shaking" if holding an icon over another, while in customization mode on RTL builds
Categories
(Firefox :: Toolbars and Customization, defect, P1)
Firefox
Toolbars and Customization
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | wontfix |
firefox56 | --- | fixed |
firefox57 | --- | verified |
People
(Reporter: itiel_yn8, Assigned: Gijs)
References
Details
(Keywords: regression, Whiteboard: [photon-structure])
Attachments
(2 files)
285.29 KB,
image/gif
|
Details | |
59 bytes,
text/x-review-board-request
|
jaws
:
review+
lizzard
:
approval-mozilla-beta+
|
Details |
Using latest Nightly. See attached demo. Not sure if this is an intended behaviour.
Assignee | ||
Comment 1•7 years ago
|
||
Is this actually a regression? I kind of doubt it...
Flags: needinfo?(itiel_yn8)
(In reply to :Gijs from comment #1) > Is this actually a regression? I kind of doubt it... Well, I didn't think about whether this is a regression or not, but now that you've mentioned it, I just checked with mozregression and yes, it is a regression, though I don't think this is really the culprit: 2017-08-04T18:52:13: DEBUG : Using url: https://hg.mozilla.org/integration/mozilla-inbound/json-pushes?changeset=c0318c8fa15e76f6eaab2df21235b196f64187b2&full=1 2017-08-04T18:52:14: DEBUG : Found commit message: Backed out changeset 502f442198b6 (bug 1360248) for build bustage in WasmSignalHandlers.cpp, at least on Windows 2012. r=backout on a CLOSED TREE 2017-08-04T18:52:14: INFO : The bisection is done. 2017-08-04T18:52:15: INFO : Stopped The regression range is from 2017-05-10 to 2017-05-11, and happens only on RTL builds it seems.
Flags: needinfo?(itiel_yn8)
Has Regression Range: --- → yes
Keywords: regression
Summary: Toolbar icons "shaking" if holding an icon over another, while in customization mode → Toolbar icons "shaking" if holding an icon over another, while in customization mode on RTL builds
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to ItielMaN from comment #2) > The regression range is from 2017-05-10 to 2017-05-11, and happens only on > RTL builds it seems. Can this be reproduced on an LTR build when switching the direction of english to be RTL via the relevant about:config pref? The regression window doesn't look like it's correct...
Flags: needinfo?(itiel_yn8)
Assignee | ||
Comment 4•7 years ago
|
||
(In reply to :Gijs from comment #3) > (In reply to ItielMaN from comment #2) > > The regression range is from 2017-05-10 to 2017-05-11, and happens only on > > RTL builds it seems. > > Can this be reproduced on an LTR build when switching the direction of > english to be RTL via the relevant about:config pref? The regression window > doesn't look like it's correct... Looks like yes...
Flags: needinfo?(itiel_yn8)
Assignee | ||
Comment 5•7 years ago
|
||
I broke this. :-(
Blocks: 1354082
Whiteboard: [photon-structure][triage]
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Iteration: --- → 57.1 - Aug 15
status-firefox55:
--- → wontfix
status-firefox56:
--- → affected
status-firefox57:
--- → affected
Flags: qe-verify+
Priority: -- → P1
QA Contact: gwimberly
Whiteboard: [photon-structure][triage] → [photon-structure]
Version: unspecified → Trunk
Comment hidden (mozreview-request) |
Assignee | ||
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8894470 [details] Bug 1387182 - fix border (empty space) compensation when dragging items in customize mode in RTL, https://reviewboard.mozilla.org/r/165646/#review170718 ::: browser/components/customizableui/CustomizeMode.jsm:1714 (Diff revision 1) > let itemRect = this._dwu.getBoundsWithoutFlushing(dragOverItem); > let dropTargetCenter = itemRect.left + (itemRect.width / 2); > let existingDir = dragOverItem.getAttribute("dragover"); > + let dirFactor = this._dir == "ltr" ? 1 : -1; > if (existingDir == "before") { > - dropTargetCenter += (parseInt(dragOverItem.style.borderInlineStartWidth) || 0) / 2; > + dropTargetCenter += (parseInt(dragOverItem.style.borderInlineStartWidth) || 0) / 2 * dirFactor; The 'start' side border is on the right in RTL, so this means the center is further to the *left* which means the x coordinate should *decrease* by the border amount, not *increase*. The inverse applies to the 'end' side border in the else block, of course.
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8894470 [details] Bug 1387182 - fix border (empty space) compensation when dragging items in customize mode in RTL, https://reviewboard.mozilla.org/r/165646/#review170892
Attachment #8894470 -
Flags: review?(jaws) → review+
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/845dd7955f8a fix border (empty space) compensation when dragging items in customize mode in RTL, r=jaws
Comment 10•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/845dd7955f8a
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Updated•7 years ago
|
Flags: qe-verify+
Comment 12•7 years ago
|
||
Managed to reproduce the issue on an affected build (Firefox 57.0a1), using the steps to reproduce from Comment 0, on Windows 10 X 64 bit. 1387182 is fixed & verified. build ID: 20170809100326 [bugday-20170809]
Updated•7 years ago
|
Assignee | ||
Comment 13•7 years ago
|
||
Comment on attachment 8894470 [details] Bug 1387182 - fix border (empty space) compensation when dragging items in customize mode in RTL, Approval Request Comment [Feature/Bug causing the regression]: bug 1354082 [User impact if declined]: broken drag/drop behaviour for RTL users [Is this code covered by automated tests?]: yes, but they don't run in RTL which is why this wasn't caught [Has the fix been verified in Nightly?]: yup [Needs manual test from QE? If yes, steps to reproduce]: yeah, 1. use an RTL build or, in about:config, set intl.uidirection to 1 and restart 2. drag 1 button in the toolbar across other buttons in the toolbar once you drag a button to both the left and right edge of another button (without dropping it in the middle) while the button is animating, this bug happens [List of other uplifts needed for the feature/fix]: n/a [Is the change risky?]: no [Why is the change risky/not risky?]: it's tiny, well-understood, there's a lot of beta left, and the change will only affect RTL which is broken-ish anyway. To be honest, before bug 1354082 the behaviour was also broken in RTL, just differently (we didn't make space on the correct side of the button making it awkward to dnd correctly). So with bug 1354082 and this bug fixed, the behaviour is actually reasonable now (yay!) [String changes made/needed]: nope
Attachment #8894470 -
Flags: approval-mozilla-beta?
Comment 14•7 years ago
|
||
Comment on attachment 8894470 [details] Bug 1387182 - fix border (empty space) compensation when dragging items in customize mode in RTL, Fix for odd behavior in customization menu for RTL, let's uplift this to beta.
Attachment #8894470 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 15•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/3fe3a48650fe
Updated•7 years ago
|
status-firefox-esr52:
--- → unaffected
Updated•7 years ago
|
Flags: qe-verify+
Updated•7 years ago
|
Flags: qe-verify+
You need to log in
before you can comment on or make changes to this bug.
Description
•