#4100 closed defect (fixed)
trying to save a modified-elsewhere ticket loses new data
Reported by: | Owned by: | Christian Boos | |
---|---|---|---|
Priority: | high | Milestone: | 0.11 |
Component: | ticket system | Version: | 0.10 |
Severity: | major | Keywords: | WorkFlow conflict |
Cc: | bboisvert@…, gay@…, thom.pischke@…, jpwasp@…, dkg-debian.org@… | Branch: | |
Release Notes: | |||
API Changes: | |||
Internal Changes: |
Description
I spent a while adding a new comment to a ticket, but when I clicked "submit" I got a page informing me that the ticket had since been updated, and I'd have to reload it and re-enter my changes. Sure enough, because trac sends pages as no-cache, the browser had tossed out my changes when I pressed the back-arrow.
I marked the severity up on this ticket because I see a loss of data as a pretty serious problem. It wouldn't even be so bad if trac had merely presented my data to me so I could copy-paste it back. An optimal solution would be trac returning me to the newly-updated ticket page with a big error message telling me that the save failed because someone else updated the ticket, and then ALSO auto-fill my changes into the appropriate boxes so I could verify and then just hit the save button again. No data loss, easy path to the most-oft-desired output.
Attachments (0)
Change History (24)
comment:1 by , 18 years ago
Keywords: | WorkFlow conflict added |
---|---|
Milestone: | → 0.11 |
comment:2 by , 18 years ago
Cc: | added |
---|
I'm with cboos. I wrote the original patch, and will freely admit it was quite hackish. It did, however, prevent the data loss, which means it was sufficient. Much better to have the errors come back to the preview/form page itself though, especially if you have multiple collision fields to resolve.
Big vote from me for this to be fixed for real.
comment:3 by , 18 years ago
#4739 raised a valid point that editing conflicts should not be reported as a "500" error. The HTTP code "409 Conflict" would be appropriate:
10.4.10 409 Conflict
The request could not be completed due to a conflict with the current state of the resource. This code is only allowed in situations where it is expected that the user might be able to resolve the conflict and resubmit the request. The response body SHOULD include enough information for the user to recognize the source of the conflict. Ideally, the response entity would include enough information for the user or user agent to fix the problem; however, that might not be possible and is not required.
Conflicts are most likely to occur in response to a PUT request. For example, if versioning were being used and the entity being PUT included changes to a resource which conflict with those made by an earlier (third-party) request, the server might use the 409 response to indicate that it can't complete the request. In this case, the response entity would likely contain a list of the differences between the two versions in a format defined by the response Content-Type.
follow-up: 6 comment:4 by , 18 years ago
Cc: | added |
---|
I'd like to add my name to the chorus of people upset about this. I have been bitten by this data loss several times already and lost tremendous amounts of work because you have broken the Back button.
It seems to me there are 2 methods of fixing this:
- just let the user handle this (by not setting No-Cache you don't break the Back button, so I could at least get back to all the text that I just lost and copy and paste; Trac forms are not any more important that any other web-based form, there is no reason to set No-Cache on the page in IMHO; in fact it violates traditional web rules for performance, let me use my browser cache)
- actually handle collisions (fwiw, bugzilla does a nice job of handling collisions and offering to you let the user decide to throw away their changes or override the other person's changes.)
I think you should just fix #1. The simple solution (and what users expect to work) would at least provide a workaround, without having to introduce a complex conflict resolution model
comment:5 by , 18 years ago
You might also want to consider that many of these so-called collisions are just going to be users adding new comments. It's really not that hard to run a diff on the ticket (since you already do it for the changelog), present the user with an error message explaining the conflicts, possibly highlighting the mismatched fields in a new css color/class, and auto-filling in with the expected data.
Like the others have said, I'd also be happy with at least a quick fix, but the larger fix is a huge step in the direction of better usability.
comment:6 by , 18 years ago
Replying to gay@recipezaar.com:
by not setting No-Cache you don't break the Back button
Can you explain this? In the past I've seen problems with IE not saving your form contents if you click Back, but this was a general problem and not related to Trac. However, I just tested this in Firefox and IE 6 an neither has a problem going Back after getting the collision error.
follow-up: 8 comment:7 by , 18 years ago
Well, I can't be certain it is the No-Cache that breaks it, I was going off what someone else said in the duplicate bug for this, but it makes sense. Either way, it happens to me in Firefox every time (mac & pc). Type long comment, submit changes, get error, click back, no comments in field. I am on version 0.10.3
comment:8 by , 18 years ago
Replying to gay@recipezaar.com:
it happens to me in Firefox every time (mac & pc). Type long comment, submit changes, get error, click back, no comments in field.
I've never had a problem with Firefox losing the comments going back, so there may be something else going on here. Please try the MailingList to get some feedback on figuring this out. I'd recommend installing an extension like LiveHTTPHeaders or TamperData to get a trace of the HTTP headers since this infomation could be useful in debugging this.
comment:9 by , 18 years ago
fyi, this happens to me with both firefox 1.5 and 2.0 under many different machines/distros in linux. Perhaps it's the fact that I've enabled "compare against cache" for every page load instead of the default once-per-session. It still doesn't excuse the fact that trac brings up a hard error with no workaround for something as simple as notifying the user that someone else has touched the ticket in the meantime.
follow-up: 11 comment:10 by , 18 years ago
When I check response headers in firebug, every page is set
Cache-Control must-revalidate Expires Fri, 01 Jan 1999 00:00:00 GMT
This is the problem, each page is refreshing on every view.
another instance, less severe, to prove this is not just a problem with comments field
- click a link from a notification, which will take you directly to a comment, e.g. <http://trac.edgewall.org/ticket/4100#comment:9>
- scroll up the page so that comment is no longer in view and the click a link to go elsewhere
- click the Back button on your browser
results you will go back to the page but the page will be scrolled back to the comment again, NOT where you last left it. this is further proof that the page is setting "Cache-control: must-revalidate" when it shouldn't. it is overriding traditional browser behavior. this would be correct behavior for a bank website for example, but trac shouldn't be refreshing every single page view.
performance degrades as well, as all js files are reloaded on each page view.
comment:11 by , 18 years ago
Replying to gay@recipezaar.com:
When I check response headers in firebug, every page is set
Cache-Control must-revalidate Expires Fri, 01 Jan 1999 00:00:00 GMT
Trac only sends an "Expires" header on error pages so that the browser doesn't cache an error message. The "must-revalidate" should affect caching proxies, but I don't think it should have an affect on the web browser, and in my experience it has not with any Mozilla-based browser. If you're seeing an "Expires" header added to every page not just error pages please check that there's not a proxy between you and Trac that's adding this.
- click a link from a notification, which will take you directly to a comment, e.g. <http://trac.edgewall.org/ticket/4100#comment:9>
- scroll up the page so that comment is no longer in view and the click a link to go elsewhere
- click the Back button on your browser
This also works as-expected for me.
performance degrades as well, as all js files are reloaded on each page view.
This is not the case. Trac does not send Cache-Control headers when serving static files from disk. They are served with an appropriate Last-Modified header, and it checks the If-Modified-Since header before serving the file and sends a 304 if necessary.
comment:12 by , 18 years ago
Cc: | added |
---|
Our team has lost countless hours to this problem. It just happened to me again, after spending half an hour entering a NEW TICKET! There was some problem with the cookie… Whatever - it's not Ok to just throw away everything I just typed!
Our team uses everything from linux/firefox to windows/IE6, so this problem is not limited just to one browser or OS. We're in the habit of ctrl-c'ing everything before submission, because this happens SO often. But sometimes we forget, or think we're safe because how can anything go wrong entering a new ticket? Oops.
Please fix this!!!
comment:13 by , 18 years ago
Ok, I think that while waiting for the ideal solution, we should at least provide a dump of the submitted data (for ticket changes, ticket submissions and probably wiki edits as well).
comment:14 by , 18 years ago
Cc: | added |
---|
I maintain multiple Trac installations and I've come across this and I've received complaints from developers, and since I fought to convince people to switch from Visual SourceSafe and nothing to Subversion and Trac it looks bad on Trac to just lose data on this (I think the problem is worse on https where Firefox doesn't cache anything). It's been over a year now; I realize the quick fix is a hack but please, at minimum, just put the comment or wiki page text content in a textbox on the error page or something else.
Although, we have resorted to other resolutions. If I've done any significant edit I always copy before submitting, and if editing wiki I will save/update the page very often to lose only the minimal possible. However, not everyone on my team follows this and sometimes 30-60 min of wiki work might be lost, especially if they haven't been bitten by this bug before.
comment:15 by , 18 years ago
follow-up: 18 comment:16 by , 18 years ago
comment:18 by , 18 years ago
Replying to thomas@apestaart.org:
The 90's are over.
This is why I wrote if you can. This is a workaround.
comment:19 by , 18 years ago
Priority: | normal → high |
---|
comment:20 by , 18 years ago
Has this been fixed?
Replying to ibexris@gmail.com:
I spent a while adding a new comment to a ticket, but when I clicked "submit" I got a page informing me that the ticket had since been updated, and I'd have to reload it and re-enter my changes. Sure enough, because trac sends pages as no-cache, the browser had tossed out my changes when I pressed the back-arrow.
I marked the severity up on this ticket because I see a loss of data as a pretty serious problem. It wouldn't even be so bad if trac had merely presented my data to me so I could copy-paste it back. An optimal solution would be trac returning me to the newly-updated ticket page with a big error message telling me that the save failed because someone else updated the ticket, and then ALSO auto-fill my changes into the appropriate boxes so I could verify and then just hit the save button again. No data loss, easy path to the most-oft-desired output.
comment:21 by , 18 years ago
Cc: | added |
---|
This is also an important issue for installations i support.
If there's a reasonable patch against the stable version which i can test, i'd be happy to try it out and report back.
comment:22 by , 18 years ago
Owner: | changed from | to
---|
comment:23 by , 18 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Going back to ticket edit with an error message seems OK, along with keeping the comment (and the replyto info). But overwriting the ticket field changes seems to be a bad idea. Rather, I think that having the conflicting changes highlighted in some way would be preferable.
There's already an open ticket with a patch (#2618), which nearly made it for 0.10, but now I think this should rather be redone the "WorkFlow way", i.e. by displaying the errors in the ticket preview itself rather than on a custom error page.