Bug 182551 – webkit-patch upload should ask for confirmation before creating a new bug

• Component: New Bugs
• Status: RESOLVED FIXED

[Ross Kirsling]

webkit-patch upload should ask for confirmation before creating a new bug

[Ross Kirsling]

Created attachment 333216 [details]
Patch

[Daniel Bates]

Comment on attachment 333216 [details]
Patch

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

> Tools/ChangeLog:7
> +

Please explain why you are making this change. I suspect it is because you inadvertently typed a non-digit when webkit-patch upload prompted for a bug number or title and webkit-patch interpreted this as a bug title as opposed to a mistyped bug number.

[Daniel Bates]

Comment on attachment 333216 [details]
Patch

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

> Tools/Scripts/webkitpy/tool/steps/promptforbugortitle.py:44
> +            if not self._tool.user.confirm("Are you sure you want to create a new bug?", default="n"):
> +                self._exit(1)

As indicated by the EWS bot this breaks unit tests, which run non-interactively. We need to take a similar approach as the CheckStyle [1] step and re-raise when running non-interactive (i.e. self._options.non_interactive returns true).

[1] <https://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/tool/steps/checkstyle.py?rev=228185#L68>

[Ross Kirsling]

Created attachment 333227 [details]
Patch

[Ross Kirsling]

I hope the way I fixed the unit test is acceptable:
1. The `catch` here is essentially the `else` of a type check, so I don't believe we want to re-raise so much as bypass the prompt.
2. The unit tests for "upload" don't have default options, so `non_interactive` is typically undefined. As such, I couldn't see a way to avoid tweaking the unit test file.

Let me know if you have any objections.

[EWS Watchlist]

Comment on attachment 333227 [details]
Patch

Attachment 333227 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/6389563

New failing tests:
transitions/transition-display-property.html

[EWS Watchlist]

Created attachment 333233 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Daniel Bates]

Comment on attachment 333227 [details]
Patch

r=me

[Daniel Bates]

Comment on attachment 333227 [details]
Patch

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

> Tools/ChangeLog:9
> +        At best, this creates an undeletable junk bug with an uneditable title.

You're talking about people with Bugzilla accounts without editbugs privileges that post patches to bugs they did not report or are assigned to, right?

[Daniel Bates]

Comment on attachment 333227 [details]
Patch

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

>> Tools/ChangeLog:9
>> +        At best, this creates an undeletable junk bug with an uneditable title.
> 
> You're talking about people with Bugzilla accounts without editbugs privileges that post patches to bugs they did not report or are assigned to, right?

Disregard this remark. If you do not have editbugs then you cannot only comment on bugs that you do not report or are not the assignee according to <https://www.bugzilla.org/docs/2.22/html/useradmin.html>. For completeness, you can always edit bug that you report or are the assignee regardless of editbug status.

[Daniel Bates]

(In reply to Daniel Bates from comment #10)
> Comment on attachment 333227 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=333227&action=review
> 
> >> Tools/ChangeLog:9
> >> +        At best, this creates an undeletable junk bug with an uneditable title.
> > 
> > You're talking about people with Bugzilla accounts without editbugs privileges that post patches to bugs they did not report or are assigned to, right?
> 
> Disregard this remark. If you do not have editbugs then you cannot only

*can only

[Ross Kirsling]

Created attachment 333237 [details]
Patch for landing

[WebKit Commit Bot]

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

media/modern-media-controls/tracks-support/tracks-support-show-panel-after-dragging-controls.html bug 169158 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

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

imported/w3c/web-platform-tests/fetch/api/redirect/redirect-to-dataurl.html bug 182561 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

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

media/modern-media-controls/tracks-support/tracks-support-show-panel-after-dragging-controls.html bug 169158 (authors: [e-mail] and [e-mail])
imported/w3c/web-platform-tests/fetch/api/cors/cors-redirect-preflight.any.worker.html bug 182562 (author: [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 333237 [details]
Patch for landing

Clearing flags on attachment: 333237

Committed r228217: <https://trac.webkit.org/changeset/228217>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/37299457>

[Simon Fraser (smfr)]

I don't like this. It's another annoying prompt that interrupts your workflow every time.

[Ross Kirsling]

(In reply to Simon Fraser (smfr) from comment #19)
> I don't like this. It's another annoying prompt that interrupts your
> workflow every time.

I think the ChangeLog is clear about the necessity of this...?

Say you happen to have some proprietary or personal information in your clipboard from an earlier activity, and this text includes a newline character. Now you're submitting a patch for an existing bug and intend to copy-paste the bug number from browser to terminal, but for some reason (be it clumsiness, distraction, some technical hiccup, whatever), that copy doesn't occur as you intended. At the moment of pasting, you've irrevocably published that information on the internet, quicker than you could even hope to Ctrl-C out of it.

You can argue that "bug tickets are free", but so long as their descriptions are uneditable, this change isn't a question of convenience but of security. I for one have felt a great deal of relief in the past month knowing that this problem is now prevented from occurring.