Closed Bug 1482992 Opened Last year Closed Last year

Horizontal layout glitch shown briefly when switching to Inspector

Categories

(DevTools :: Inspector, defect, P3)

62 Branch
defect

Tracking

(firefox63 fixed)

RESOLVED FIXED
Firefox 63
Tracking Status
firefox63 --- fixed

People

(Reporter: Harald, Assigned: gl)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

Nightly, 63.0a1 (2018-08-13) (64-bit), OSX

STR:
- Switch from Console to Inspector

See attached screencast (25% speed).

https://perfht.ml/2KKDiHD
Assignee: nobody → gl
Status: NEW → ASSIGNED
Priority: -- → P3
Attached patch 1482992.patchSplinter Review
The problem is that when we switch tabs, onPanelWindowResize is called and that will call onLazyPanelResize and since the inspector isn't visible, useLandscapeMode is set to false. When we switch back to the inspector, useLandscapeMode is set to true again but there is a delay. So, the solution here is to avoid calling all these handlers if the inspector is not visible.
Attachment #9002906 - Flags: review?(bgrinstead)
Comment on attachment 9002906 [details] [diff] [review]
1482992.patch

Review of attachment 9002906 [details] [diff] [review]:
-----------------------------------------------------------------

::: devtools/client/inspector/inspector.js
@@ +638,5 @@
>     * If Toolbox width is less than 600 px, the splitter changes its mode
>     * to `horizontal` to support portrait view.
>     */
>    onPanelWindowResize: function() {
> +    if (this.toolbox.currentToolId !== "inspector") {

Unless if I'm missing something, given:

1. Open inspector and make window big
2. Switch to console and make window small
3. Switch back to inspector

In this case `onPanelWindowResize` won't be called at (3), which means the inspector wouldn't flip the vert mode until a resize happens again, right?
Attachment #9002906 - Flags: review?(bgrinstead)
(In reply to Brian Grinstead [:bgrins] from comment #2)
> Comment on attachment 9002906 [details] [diff] [review]
> 1482992.patch
> 
> Review of attachment 9002906 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: devtools/client/inspector/inspector.js
> @@ +638,5 @@
> >     * If Toolbox width is less than 600 px, the splitter changes its mode
> >     * to `horizontal` to support portrait view.
> >     */
> >    onPanelWindowResize: function() {
> > +    if (this.toolbox.currentToolId !== "inspector") {
> 
> Unless if I'm missing something, given:
> 
> 1. Open inspector and make window big
> 2. Switch to console and make window small
> 3. Switch back to inspector
> 
> In this case `onPanelWindowResize` won't be called at (3), which means the
> inspector wouldn't flip the vert mode until a resize happens again, right?

I have tested this and "onPanelWindowResize" is called at (3). I couldn't find the exact code of what happens, but an inspection of the size of the inspector iframe and body element shows that it has a size of 0x0 whenever it is not selected, and resizes back to the toolbox width whenever it is selected. Reflagging for another review.
Attachment #9002906 - Flags: review?(bgrinstead)
(In reply to Gabriel [:gl] (ΦωΦ) from comment #3)
> I have tested this and "onPanelWindowResize" is called at (3). I couldn't
> find the exact code of what happens, but an inspection of the size of the
> inspector iframe and body element shows that it has a size of 0x0 whenever
> it is not selected, and resizes back to the toolbox width whenever it is
> selected. Reflagging for another review.

OK, I guess it would ultimately be a 'lazy' resize in this case, so the vertical flip would happen after a delay, much like this bug. But given that this bug is the normal case (where no flip is supposed to happen), that's still an improvement.
Comment on attachment 9002906 [details] [diff] [review]
1482992.patch

Review of attachment 9002906 [details] [diff] [review]:
-----------------------------------------------------------------

r=me since it fixes the glitch, but I'd confirm on try that it's green.
Attachment #9002906 - Flags: review?(bgrinstead) → review+
Pushed by gabriel.luong@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2da8fd86f936
Early return from the panel window resize handler if the inspector is not selected. r=bgrins
https://hg.mozilla.org/mozilla-central/rev/2da8fd86f936
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
You need to log in before you can comment on or make changes to this bug.