Bug 54163 - only erase draft comments after publish is successful
Summary: only erase draft comments after publish is successful
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Ojan Vafai
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-09 20:10 PST by Ojan Vafai
Modified: 2011-02-10 21:12 PST (History)
2 users (show)

See Also:


Attachments
Patch (2.98 KB, patch)
2011-02-09 20:12 PST, Ojan Vafai
abarth: review+
Details | Formatted Diff | Diff
publish when not logged in (93.43 KB, image/png)
2011-02-10 20:53 PST, Ojan Vafai
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ojan Vafai 2011-02-09 20:10:13 PST
only erase draft comments after publish is successful
Comment 1 Ojan Vafai 2011-02-09 20:12:19 PST
Created attachment 81916 [details]
Patch
Comment 2 Adam Barth 2011-02-10 01:21:15 PST
Comment on attachment 81916 [details]
Patch

What happens if there's a mid-air collision?
Comment 3 Ojan Vafai 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. :)
Comment 4 Adam Barth 2011-02-10 20:42:56 PST
Is the iframe visible?  I is confused.
Comment 5 Ojan Vafai 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.
Comment 6 Ojan Vafai 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.
Comment 7 Ojan Vafai 2011-02-10 20:45:59 PST
Whoops. Screenshot preparation fail.
Comment 8 Ojan Vafai 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!
Comment 9 Ojan Vafai 2011-02-10 20:48:05 PST
(In reply to comment #7)
> Whoops. Screenshot preparation fail.

Ahh. Sorry for the noise.
Comment 10 Ojan Vafai 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.
Comment 11 Adam Barth 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.
Comment 12 Ojan Vafai 2011-02-10 20:53:34 PST
Created attachment 82096 [details]
publish when not logged in
Comment 13 Ojan Vafai 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.
Comment 14 Adam Barth 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.
Comment 15 Ojan Vafai 2011-02-10 21:12:56 PST
Committed r78310: <http://trac.webkit.org/changeset/78310>