Closed
Bug 438964
Opened 16 years ago
Closed 16 years ago
Closing Edit Event window without modifications prompts Save Event dialog if event is on a remote calendar and it wasn't saved twice.
Categories
(Calendar :: Internal Components, defect)
Calendar
Internal Components
Tracking
(Not tracked)
RESOLVED
FIXED
1.0b1
People
(Reporter: bv1578, Assigned: Fallen)
Details
Attachments
(3 files)
1.10 KB,
patch
|
cmtalbert
:
review+
|
Details | Diff | Splinter Review |
866 bytes,
patch
|
Fallen
:
review+
|
Details | Diff | Splinter Review |
463 bytes,
patch
|
dbo
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; it; rv:1.8.1.14) Gecko/20080404 Firefox/2.0.0.14 Build Identifier: Lightning 0.9pre (2008061118) - Thunderbird 2.0.0.14 (20080421) An event on remote calendar (new or after a change) needs to be saved twice, in two different "edit session" with 'save and close' button (Edit Event window), 'Save' button (Save Event dialog), enter key or a combination of two of them, otherwise Lightning prompts Save Event dialog every time the Edit Event window is closed without changes. Reproducible: Always Steps to Reproduce: 1. Create a new event on a remote calendar and save it with "save and close" button or enter key; 2. edit the event and close the Edit Event window without any change. Actual Results: Save Event dialog appears. 3a. press "don't save button" -> you saved once: next time you edit you are in the situation of point 2. 3b. press "save" button -> you saved twice: next time you edit, Save Event dialog doesn't appear when you close the Edit Event window without any change. Other case: 2. edit the event and press "save and close" button without any change -> you saved twice: next time you edit, Save Event dialog doesn't appear when you close the Edit Event window without any change. You get the same situation of point 1 if you change the event and save it. Expected Results: Save Event dialog shouldn't appear because no change has been done. I verified this behavior with Google calendars and with provider for Google calendar 0.4.
Comment 1•16 years ago
|
||
Does this happens only with gdata provider? Or also with e.g. local storage, ics or caldav provider?
Local calendars have no problem; ics calendars subscribed as network calendars but hosted on my computer (e.g. location file:///D:/test.ics) have no problem; I can't try caldav provider. Forgot: error console reports no error.
Updated•16 years ago
|
Component: General → Provider: GData
QA Contact: general → gdata-provider
Comment 3•16 years ago
|
||
I'm finding this behavior on the locally saved Sunbird v0.8 calendar too. These are my steps. Create an event which repeats. Choose to edit the event, select "This Occurrence Only", edit the event. Select Save. The edit is not reflected in the calendar. Choose to edit the event a second time, edit the event, Choose save and close and the changes are now reflected on the calendar. If one makes no changes and selects save and close then edits it again, the changes are reflected on the calendar. It seems that the first edit removed the event from the recurrence and the second edit permits changes to the item.
Comment 4•16 years ago
|
||
(In reply to comment #3) Mike, that's Bug 427276 (fixed for Sunbird 0.9).
Assignee | ||
Comment 5•16 years ago
|
||
I was able to reproduce. I must say I have been seeing and ignoring this bug a while ;-) This fixes, I heard rfc2445 doesn't permit empty properties so this fix is valid.
Assignee: nobody → philipp
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #337504 -
Flags: review?
Attachment #337504 -
Flags: review? → review+
Comment on attachment 337504 [details] [diff] [review] Fix - v1 r=ctalbert. This looks good. There was some discussion of this on IRC, and we need to watch out for the case where this property can be undefined (if that is possible) Undefined should probably throw, but we can address that in another bug if need be. For reference, the IRC discussion: [12:05pm] Fallen: mvl (or anyone else): does anything speak against just checking if (aValue) { /* line 421 */ } else { /* line 419 */ } in http://mxr.mozilla.org/mozilla1.8/source/calendar/base/src/calItemBase.js?mark=418-422#400 ? This would change that if an empty property is set (i.e item.setProperty("LOCATION", "")) then it would not be set on the item, and afaik an empty property is not valid per rfc2445. [12:05pm] Cedric: sipaq : ok [12:06pm] Fallen: dmose,ctalbert: ^^ [12:07pm] ludovic joined the chat room. [12:07pm] sipaq: Cedric: did the mail reach you yet? [12:07pm] Cedric: sipaq: yes, I got it [12:08pm] ctalbert: Fallen, looking at it, need a bit of time [12:08pm] IanN joined the chat room. [12:10pm] Fallen: ctalbert: thanks! [12:11pm] dmose: Fallen: interestingly, you asked a related question when originally revbiewing: https://bugzilla.mozilla.org/show_bug.cgi?id=368976#c16 [12:12pm] dmose: Fallen: what you're suggesting seems ok to me, assuming that statement about empty properties being invalid is true [12:12pm] dmose: actually, that question wasn't as related as i thought, i see now [12:14pm] mvl: Fallen: pong [12:17pm] Fallen: mvl: what I asked above [12:17pm] mvl: Fallen: I wonder what should happen for item.setProperty("LOCATION", undefined) [12:17pm] mvl: I think that should also delete, thus your suggestion sounds ok [12:19pm] Siwy4444 left the chat room. (Connection reset by peer) [12:20pm] dmose: unless undefined is likely to be a bug [12:20pm] dmose: in which case maybe it should throw
Assignee | ||
Comment 7•16 years ago
|
||
Checked in on HEAD and MOZILLA_1_8_BRANCH -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
OS: Windows XP → All
Hardware: PC → All
Resolution: --- → FIXED
Target Milestone: --- → 0.9
Assignee | ||
Comment 8•16 years ago
|
||
(and on SUNBIRD_0_9_BRANCH, my bookmarks just don't do that automatically ;-)
Comment 9•16 years ago
|
||
(In reply to comment #6) > ? This would change that if an empty property is set (i.e > item.setProperty("LOCATION", "")) then it would not be set on the item, and > afaik an empty property is not valid per rfc2445. Is this really the case? Being picky, reading <http://tools.ietf.org/rfcmarkup?doc=draft-ietf-calsify-rfc2445bis-07#section-3.1> value is *VALUE-CHAR, thus there need not be any value.
Assignee | ||
Comment 10•16 years ago
|
||
iirc, libical inserts an X-LIC-ERROR and removes the property if you insert an empty property.
Updated•16 years ago
|
Component: Provider: GData → Internal Components
Flags: in-testsuite?
QA Contact: gdata-provider → base
Comment 11•16 years ago
|
||
Comment on attachment 337504 [details] [diff] [review] Fix - v1 > setProperty: function (aName, aValue) { > if (aName == "LAST-MODIFIED") { > this.mDirty = false; > } else { > this.modify(); > } >- if (aValue === null) { >- this.deleteProperty(aName); >- } else { >+ if (aValue) { > this.mProperties.setProperty(aName.toUpperCase(), aValue); >+ } else { >+ this.deleteProperty(aName); I think we should head for a different solution, because this change effectively hinders setting/creating properties of value zero, e.g. PRIORITY:0 (probably no problem since it's equivalent with no PRIORITY) X-MOZ-MY:0 (will not be roundtripped) I suspect this change is the case for the currently broken unit tests (ics roundtrip breaks on PRIORITY).
Updated•16 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 13•16 years ago
|
||
Can we mark this bug FIXED again?
Assignee | ||
Comment 14•16 years ago
|
||
Ah yes, sorry about that.
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 15•16 years ago
|
||
I again have to reopen this bug. I stumbled upon the exact same bug and noticed that: isNaN("") == false which in turn causes empty properties to be added.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 16•16 years ago
|
||
Attachment #341293 -
Flags: review?(daniel.boelzle)
Updated•16 years ago
|
Attachment #341293 -
Flags: review?(daniel.boelzle) → review+
Assignee | ||
Comment 17•16 years ago
|
||
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/bbc182397949> -> FIXED
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Target Milestone: 0.9 → 1.0
Assignee | ||
Comment 18•13 years ago
|
||
These bugs are likely targeted at Lightning 1.0b1, not Lightning 1.0. If this change was done in error, please adjust the target milestone to its correct value. To filter on this bugspam, you can use "lightning-10-target-move".
Target Milestone: 1.0 → 1.0b1
Updated•6 years ago
|
Flags: in-testsuite?
You need to log in
before you can comment on or make changes to this bug.
Description
•