Bug 185369 – Web Inspector: Console drawer resizing is broken when console prompt has >1 line of code

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Nikita Vasilyev]

Created attachment 339701 [details]
[Video] Screencast

Steps:
1. Open Elements tab.
2. Press Escape key to open the console drawer.
3. Type `function a() {`, press Return several times
4. Try to resize the console drawer now.

Notes:
Console drawer resizing works as expected when it's empty or has only one line of code.

This is not a recent regression.

[Radar WebKit Bug Importer]

<rdar://problem/40013202>

[Matt Baker]

Created attachment 339783 [details]
Patch

[Matt Baker]

Created attachment 339784 [details]
[Video] With fix

[Nikita Vasilyev]

(In reply to Matt Baker from comment #2)
> Created attachment 339783 [details]
> Patch

Works as expected. The code looks good.

At 0:10 of the video you can see there's some z-index issues with the right sidebar tabs, but it's a separate issue.

[Devin Rousso]

Comment on attachment 339783 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=339783&action=review

r=me

Nice fix!

> Source/WebInspectorUI/UserInterface/Views/ConsoleDrawer.js:125
> +        let quickConsoleHeight = window.innerHeight - (this.element.totalOffsetTop + this.height);

NIT: I'd prefer this be rewritten without the parenthesis, as I first thought that the `this.element.totalOffsetTop` and `this.height` were somehow "related".  A comment might also help explaining that "this gets the full height of the console prompt by taking into account the current position of the console drawer and its height with respect to the entire inspector area, so that the remainder is the height of the console prompt", or something like that.

Shame that there's no easy way of getting the height from the `WI.ConsolePrompt` and passing it in here.  This method works just fine though.

[Matt Baker]

(In reply to Devin Rousso from comment #5)
> Comment on attachment 339783 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=339783&action=review
> 
> r=me
> 
> Nice fix!
> 
> > Source/WebInspectorUI/UserInterface/Views/ConsoleDrawer.js:125
> > +        let quickConsoleHeight = window.innerHeight - (this.element.totalOffsetTop + this.height);
> 
> NIT: I'd prefer this be rewritten without the parenthesis, as I first
> thought that the `this.element.totalOffsetTop` and `this.height` were
> somehow "related".

The parenthesis are needed. Arithmetic operators are left-associative:

jsc:
>>> 1 - 1 + 1
1

>>> 1 - (1 + 1)
-1

[Devin Rousso]

(In reply to Matt Baker from comment #6)
> (In reply to Devin Rousso from comment #5)
> > Comment on attachment 339783 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=339783&action=review
> > 
> > r=me
> > 
> > Nice fix!
> > 
> > > Source/WebInspectorUI/UserInterface/Views/ConsoleDrawer.js:125
> > > +        let quickConsoleHeight = window.innerHeight - (this.element.totalOffsetTop + this.height);
> > 
> > NIT: I'd prefer this be rewritten without the parenthesis, as I first
> > thought that the `this.element.totalOffsetTop` and `this.height` were
> > somehow "related".
> 
> The parenthesis are needed. Arithmetic operators are left-associative:
> 
> jsc:
> >>> 1 - 1 + 1
> 1
> 
> >>> 1 - (1 + 1)
> -1
Oh lol, I should've specified that you'd have to flip the sign as well.

    let quickConsoleHeight = window.innerHeight - this.element.totalOffsetTop - this.height;

[WebKit Commit Bot]

Comment on attachment 339783 [details]
Patch

Clearing flags on attachment: 339783

Committed r231527: <https://trac.webkit.org/changeset/231527>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.