Bug 166935 – Web Inspector: popovers shouldn't be dismissed when Web Inspector window is dragged

• Component: Web Inspector
• Status: RESOLVED FIXED

[Blaze Burg]

This is supremely annoying if you are using a CSS property editor and lose your changes.

[Radar WebKit Bug Importer]

<rdar://problem/29972986>

[Devin Rousso]

Created attachment 301661 [details]
Patch

[Devin Rousso]

(In reply to comment #2)
> Created attachment 301661 [details]
> Patch

I tested this on <https://webkit.org>.  For each of the following, find a color inline swatch somewhere in the CSS - Rules sidebar and open it before doing what's listed:

- Click one of the dock configuration buttons (detach, dock right, dock bottom)
- When undocked, resize the inspector window from any side
- When undocked, drag the inspector window from empty space in the toolbar area
- When docked right, drag the resizer on the left side of the inspector window
- When docked bottom, drag the resizer on the top side of the inspector window
- Pressing ⌘⇧D to toggle to the last docked configuration

None of the above should dismiss the color picker popover.

[Devin Rousso]

Created attachment 301676 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 301676 [details]
Patch

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

r=me

You mentioned:

- When docked right, drag the resizer on the left side of the inspector window
- When docked bottom, drag the resizer on the top side of the inspector window
- Pressing ⌘⇧D to toggle to the last docked configuration

I'd actually expect resizes to dismiss or maybe redraw / reposition the popover. Is that still the case?

> Source/WebInspectorUI/UserInterface/Views/Popover.js:173
> -            if (!this._element.contains(event.target) && !event.target.enclosingNodeOrSelfWithClass(WebInspector.Popover.IgnoreAutoDismissClassName))
> +            if (!this._element.contains(event.target) && !event.target.enclosingNodeOrSelfWithClass(WebInspector.Popover.IgnoreAutoDismissClassName)
> +                && !event[WebInspector.Popover.EventPreventDismissSymbol]) {
>                  this.dismiss();
> +            }

Style: I think all on one line is fine, or breaking it up on multiple lines:

    if (this._element.contains(event.target))
        break;
    if (event.target.enclosingNodeOrSelfWithClass(WebInspector.Popover.IgnoreAutoDismissClassName))
        break;
    if (event[WebInspector.Popover.EventPreventDismissSymbol])
        break;
    this.dismiss();
    break;

[Blaze Burg]

(In reply to comment #5)
> Comment on attachment 301676 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=301676&action=review
> 
> r=me
> 
> You mentioned:
> 
> - When docked right, drag the resizer on the left side of the inspector
> window
> - When docked bottom, drag the resizer on the top side of the inspector
> window
> - Pressing ⌘⇧D to toggle to the last docked configuration
> 
> I'd actually expect resizes to dismiss or maybe redraw / reposition the
> popover. Is that still the case?

Note that this bug (per the title) is about dragging. For resizing, I think dismissal is fine.

[Joseph Pecoraro]

We should match system behavior. System behavior doesn't dismiss popovers (though it does allow repositioning them to more optimal positions). That was our existing behavior and it seems that is what Devin says still happens. I just wanted to make sure that was still the case.

[Devin Rousso]

Comment on attachment 301676 [details]
Patch

(In reply to comment #7)
> We should match system behavior. System behavior doesn't dismiss popovers
> (though it does allow repositioning them to more optimal positions). That
> was our existing behavior and it seems that is what Devin says still
> happens. I just wanted to make sure that was still the case.

So it was already the case that resizing when undocked would not dismiss, but instead just reposition the popover.  I have extended this to also work when docked (both right and bottom), meaning that whenever the window is moved (when undocked) or resized the popover just recalculates its position.

[WebKit Commit Bot]

Comment on attachment 301676 [details]
Patch

Clearing flags on attachment: 301676

Committed r212427: <http://trac.webkit.org/changeset/212427>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.