Bug 169856

Summary: Web Inspector: Clicking DOM breakpoint marker should enable/disable breakpoints
Product: WebKit Reporter: Matt Baker <mattbaker>
Component: Web InspectorAssignee: Matt Baker <mattbaker>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, inspector-bugzilla-changes, joepeck, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch for landing none

Matt Baker
Reported 2017-03-18 16:41:35 PDT
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.
Attachments
Patch (2.78 KB, patch)
2017-03-18 16:44 PDT, Matt Baker 		no flags
DetailsFormatted DiffDiff
Patch for landing (2.84 KB, patch)
2017-03-21 12:37 PDT, Matt Baker 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2017-03-18 16:42:02 PDT
<rdar://problem/31133090>
Matt Baker
Comment 2 2017-03-18 16:44:56 PDT
Created attachment 304886 [details] Patch
Joseph Pecoraro
Comment 3 2017-03-20 16:51:00 PDT
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
Comment 4 2017-03-20 17:14:10 PDT
(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
Comment 5 2017-03-20 17:17:11 PDT
> 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
Comment 6 2017-03-21 12:35:51 PDT
(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
Comment 7 2017-03-21 12:37:24 PDT
Created attachment 305012 [details] Patch for landing
WebKit Commit Bot
Comment 8 2017-03-22 10:40:46 PDT
The commit-queue encountered the following flaky tests while processing attachment 305012 [details]: media/track/track-in-band-style.html bug 153143 (authors: dgorbik@apple.com, eric.carlson@apple.com, and jer.noble@apple.com) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 9 2017-03-22 10:41:15 PDT
Comment on attachment 305012 [details] Patch for landing Clearing flags on attachment: 305012 Committed r214256: <http://trac.webkit.org/changeset/214256>
WebKit Commit Bot
Comment 10 2017-03-22 10:41:19 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.