Closed Bug 1468953 Opened Last year Closed Last year

When docking the Animation Inspector to the side, panel resizing works incorrectly in some cases

Categories

(DevTools :: Inspector: Animations, defect, P3, minor)

defect

Tracking

(firefox61 disabled, firefox62 fixed, firefox63 verified)

VERIFIED FIXED
Firefox 63
Tracking Status
firefox61 --- disabled
firefox62 --- fixed
firefox63 --- verified

People

(Reporter: danibodea, Assigned: daisuke)

References

(Blocks 2 open bugs)

Details

Attachments

(3 files)

Attached image example.png
[Environment:]
Windows 10, Ubuntu 16.04, Mac OS X 10.13
Nightly 62.0a1 (2018-06-15)

[Steps:]
1. Open Firefox Nightly and load https://rawgit.com/dadaa/3b73f847427025b51ba1ab7333013d0c/raw/77f3f0bb884875a179c3407f73bf8a8dd54751c9/doc_custom_playback_rate.html
2. Open Animation Inspector.
3. Select any of the two divs/nodes.
4. Dock the Developer Tools to the right or left.
5. Resize Animation Inspector panels to have similar proportions to the image attached with the name of "example.png".
6. Catch the panel border between the animation graphs panel and animation property graphs panel AND drag it up-down.

[Actual Result:]
The border jumps down when and is not synced relative to the movement of the cursor.

[Expected Result:]
The border is caught and dragged as expected.
Thank you for the reporting.
This seems like the shared component SplitBox[1] might have the problem.

[1] https://searchfox.org/mozilla-central/source/devtools/client/shared/components/splitter/SplitBox.js
Priority: -- → P3
Assignee: nobody → dakatsuka
Comment on attachment 8986998 [details]
Bug 1468953: Use node bounds in viewport instead of offsets from offsetParent.

https://reviewboard.mozilla.org/r/252260/#review259350

Makes sense! x and y are viewport coordinates, so they should be compared with the viewport coordinates of the splitter container.

::: commit-message-1f039:1
(Diff revision 1)
> +Bug 1468953: Use top/left position in viewport instead of clinetX/clientTop of offsetParent. r?jdescottes

nit: typo clinet -> client

::: devtools/client/shared/components/splitter/SplitBox.js:186
(Diff revision 1)
>        if (doc.dir === "rtl") {
>          endPanelControl = !endPanelControl;
>        }
>  
>        size = endPanelControl ?
> -        (node.offsetLeft + node.offsetWidth) - x :
> +        (nodeBounds.left + node.offsetWidth) - x :

maybe for consistency, use nodeBounds.width and nodeBounds.height, rather than mixing bounds and offsets?
Attachment #8986998 - Flags: review?(jdescottes) → review+
Comment on attachment 8986998 [details]
Bug 1468953: Use node bounds in viewport instead of offsets from offsetParent.

https://reviewboard.mozilla.org/r/252260/#review259350

> maybe for consistency, use nodeBounds.width and nodeBounds.height, rather than mixing bounds and offsets?

Oh, yeah! Thanks Julian!
Comment on attachment 8986998 [details]
Bug 1468953: Use node bounds in viewport instead of offsets from offsetParent.

https://reviewboard.mozilla.org/r/252260/#review259350

> Oh, yeah! Thanks Julian!

Also, I'll change the commit message to reflect this change.
Maybe:
Use node bounds in viewport instead of offsets from offsetParent.
Pushed by dakatsuka@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2a91df8739c9
Use node bounds in viewport instead of offsets from offsetParent. r=jdescottes
https://hg.mozilla.org/mozilla-central/rev/2a91df8739c9
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Hello,

This issue seems fixed in latest Nightly v63.0a1 (2018-06-26) on Windows 10, Ubuntu 16.04 and Mac OS X 10.12.06.
Status: RESOLVED → VERIFIED
Can you request uplift to beta? If you think it would be a good idea, we could get this into beta 4 or 5.
Flags: needinfo?(dakatsuka)
Thanks Liz!
I will do it for this bug, and other related bugs under bug 1399830.
Flags: needinfo?(dakatsuka)
Comment on attachment 8986998 [details]
Bug 1468953: Use node bounds in viewport instead of offsets from offsetParent.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1399830
[User impact if declined]: As commented in comment 0 and attachment 8985612 [details], user can not resize the animation inspector panes correctly by Drag and Drop.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: No
[Is the change risky?]: Low
[Why is the change risky/not risky?]: This change was only a few lines and we got the Verified status from QA.
[String changes made/needed]: N/A
Attachment #8986998 - Flags: approval-mozilla-beta?
Comment on attachment 8986998 [details]
Bug 1468953: Use node bounds in viewport instead of offsets from offsetParent.

Fix verified in nightly, and this is for a new feature shipping in 62. Let's uplift for beta 5.
Attachment #8986998 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.