Bug 169856 – Web Inspector: Clicking DOM breakpoint marker should enable/disable breakpoints

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

[Matt Baker]

Summary:
Clicking DOM breakpoint marker should enable/disable breakpoints. Behavior should match the breakpoint marker context menu:

1) If one or more breakpoints is disabled, clicking enables all.
2) If all breakpoints are enabled, clicked disables all.

[Radar WebKit Bug Importer]

<rdar://problem/31133090>

[Matt Baker]

Created attachment 304886 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 304886 [details]
Patch

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

Nice! r=me

> Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:1696
> +    _statusImageClicked(event)
> +    {

The element handles contextmenu but what if a 3 button mouse user does a middle-click? We should match what regular breakpoints do, but a lot of our code does this dance:

    if (event.button !== 0 || event.ctrlKey)
        return;

[Matt Baker]

(In reply to comment #3)
> Comment on attachment 304886 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=304886&action=review
> 
> Nice! r=me
> 
> > Source/WebInspectorUI/UserInterface/Views/DOMTreeElement.js:1696
> > +    _statusImageClicked(event)
> > +    {
> 
> The element handles contextmenu but what if a 3 button mouse user does a
> middle-click? We should match what regular breakpoints do, but a lot of our
> code does this dance:
> 
>     if (event.button !== 0 || event.ctrlKey)
>         return;

It looks like line widgets for breakpoints handle mouse up, and don't do any button checks (other than to make sure a drag wasn't taking place).

[Joseph Pecoraro]

> It looks like line widgets for breakpoints handle mouse up, and don't do any
> button checks (other than to make sure a drag wasn't taking place).

Maybe we should improve that then too!

[Matt Baker]

(In reply to Joseph Pecoraro from comment #5)
> > It looks like line widgets for breakpoints handle mouse up, and don't do any
> > button checks (other than to make sure a drag wasn't taking place).
> 
> Maybe we should improve that then too!

It turns out we do make the check, in TextEditor.prototype._gutterMouseDown before we add the mouse up event listener.

[Matt Baker]

Created attachment 305012 [details]
Patch for landing

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 305012 [details]:

media/track/track-in-band-style.html bug 153143 (authors: [e-mail], [e-mail], and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 305012 [details]
Patch for landing

Clearing flags on attachment: 305012

Committed r214256: <http://trac.webkit.org/changeset/214256>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.