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

Status: VERIFIED FIXED

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

Description

[Daniel Bodea [:danibodea]]

Attachment: 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.

Comment 1

[Daniel Bodea [:danibodea]]

Attachment: video with reproduction of the panel resizing bug

Comment 2

[Daisuke Akatsuka (:daisuke)]

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

Comment 3

[Daisuke Akatsuka (:daisuke)]

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7b7fe983279008ae5471aa1cf1857dcc2db14f31

Comment 4

[Daisuke Akatsuka (:daisuke)]

Attachment: Bug 1468953: Use node bounds in viewport instead of offsets from offsetParent.

Review commit: https://reviewboard.mozilla.org/r/252260/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/252260/

Comment 5

[Julian Descottes [:jdescottes]]

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?

Comment 6

[Daisuke Akatsuka (:daisuke)]

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 7

[Daisuke Akatsuka (:daisuke)]

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.

Comment 8

[Daisuke Akatsuka (:daisuke)]

Comment on attachment 8986998 [details]
Bug 1468953: Use node bounds in viewport instead of offsets from offsetParent.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/252260/diff/1-2/

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2a91df8739c9
Use node bounds in viewport instead of offsets from offsetParent. r=jdescottes

Comment 10

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/2a91df8739c9

Comment 11

[Daniel Bodea [:danibodea]]

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.

Comment 12

[Liz Henry (:lizzard)]

Can you request uplift to beta? If you think it would be a good idea, we could get this into beta 4 or 5.

Comment 13

[Daisuke Akatsuka (:daisuke)]

Thanks Liz!
I will do it for this bug, and other related bugs under bug 1399830.

Comment 14

[Daisuke Akatsuka (:daisuke)]

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

Comment 15

[Liz Henry (:lizzard)]

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.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/c1d5fc9c6abf