Bug 54163

Summary: only erase draft comments after publish is successful
Product: WebKit Reporter: Ojan Vafai <ojan>
Component: New BugsAssignee: Ojan Vafai <ojan>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, kbr
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Attachments:
Description Flags
Patch
abarth: review+
publish when not logged in none

Ojan Vafai
Reported 2011-02-09 20:10:13 PST
only erase draft comments after publish is successful
Attachments
Patch (2.98 KB, patch)
2011-02-09 20:12 PST, Ojan Vafai 		abarth: review+
DetailsFormatted DiffDiff
publish when not logged in (93.43 KB, image/png)
2011-02-10 20:53 PST, Ojan Vafai 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Ojan Vafai
Comment 1 2011-02-09 20:12:19 PST
Created attachment 81916 [details] Patch
Adam Barth
Comment 2 2011-02-10 01:21:15 PST
Comment on attachment 81916 [details] Patch What happens if there's a mid-air collision?
Ojan Vafai
Comment 3 2011-02-10 14:40:07 PST
(In reply to comment #2) > (From update of attachment 81916 [details]) > What happens if there's a mid-air collision? You'll get the mid-air collision page in an iframe. You can continue with the submit or cancel. If you cancel, the iframe will navigate back to the bug page. To get back to the review you'll need to refresh the review page, which will have all your comments safely stored. :)
Adam Barth
Comment 4 2011-02-10 20:42:56 PST
Is the iframe visible? I is confused.
Ojan Vafai
Comment 5 2011-02-10 20:44:23 PST
(In reply to comment #4) > Is the iframe visible? I is confused. Yes. It's the reviewform iframe. I'll try to upload a screenshot.
Ojan Vafai
Comment 6 2011-02-10 20:44:38 PST
Comment on attachment 81916 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=81916&action=review > Websites/bugs.webkit.org/ChangeLog:13 > + Once WebKit supports seamless iframes we should be able to avoid Dummy comment.
Ojan Vafai
Comment 7 2011-02-10 20:45:59 PST
Whoops. Screenshot preparation fail.
Ojan Vafai
Comment 8 2011-02-10 20:46:18 PST
Comment on attachment 81916 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=81916&action=review > Websites/bugs.webkit.org/code-review.js:1040 > + // This is the intial load of the review form iframe. Don't publish!
Ojan Vafai
Comment 9 2011-02-10 20:48:05 PST
(In reply to comment #7) > Whoops. Screenshot preparation fail. Ahh. Sorry for the noise.
Ojan Vafai
Comment 10 2011-02-10 20:48:57 PST
Comment on attachment 81916 [details] Patch Looks like the review form doesn't hit mid-air collissions? I just publishes anyways.
Adam Barth
Comment 11 2011-02-10 20:51:58 PST
> Looks like the review form doesn't hit mid-air collissions? I just publishes anyways. It will hit a mid-air collision if someone modifies the attachment while the review is in flight.
Ojan Vafai
Comment 12 2011-02-10 20:53:34 PST
Created attachment 82096 [details] publish when not logged in
Ojan Vafai
Comment 13 2011-02-10 20:55:07 PST
Sorry for all the noise. I wish bugzilla would let you delete comments. Anyways, it looks like you don't get a mid-air collision when there is a review conflict, it just publishes it anyways. For the case where you're not logged in though, we popup the iframe as I described. I attached a screenshot. From within the iframe you can then login and complete publishing.
Adam Barth
Comment 14 2011-02-10 20:59:24 PST
Comment on attachment 81916 [details] Patch I'm slightly scared of this misbehaving, but I like the concept. If there are bugs, we can fix them.
Ojan Vafai
Comment 15 2011-02-10 21:12:56 PST
Committed r78310: <http://trac.webkit.org/changeset/78310>
Note You need to log in before you can comment on or make changes to this bug.