416525 - Add Link for task or event does not work (URL is not saved)

Status: VERIFIED FIXED

Description

[Lars Wohlfahrt (thetux)]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.13pre) Gecko/20080208 Calendar/0.8pre 20080208

Steps to Reproduce:
1) Add Link for event or task
2) Enter URL and press OK
3) URL is shown next to the Label "Link:"
4) Press "Save and Close"

Actual Results:
1) URL is not saved.

Expected Results:
1) URL should be saved.

Comment 1

[Martin Schröder [:martinschroeder]]

Confirmed on Windows. Raising to blocking-calendar0.8?.

Comment 2

[Lars Wohlfahrt (thetux)]

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.13pre) Gecko/20080207 Calendar/0.8pre 20080207 is the last working version.

Comment 3

[Omar Bajraszewski]

Regression range:
Works with Lt 2008020719
Fails with Lt 2008020819

Comment 4

[Stefan Sitter [:ssitter]]

Confirmed as regession:
WORKS for me using Sunbird 0.8pre (2008020806)
FAILS for me using Sunbird 0.8pre (2008020820)

Checkins during regession range: http://tinyurl.com/2b6d47

Most probably caused by Bug 327780 that touched sun-calendar-event-dialog.js.

Comment 5

[Stefan Sitter [:ssitter]]

This seems to be caused by a copy paste error in attachment 302140 [details] [diff] [review]:

The call to capSupported() was removed and replaced with the methods code:

mozilla/calendar/prototypes/wcap/sun-calendar-event-dialog.js:
-function setItemProperty(item, propertyName, aValue, aCapability) {
-    var value = (aCapability && !capSupported(aCapability) ? null : aValue);

mozilla/calendar/base/src/calUtils.js:
+function setItemProperty(item, propertyName, aValue, aCapability) {
+    var isSupported = (item.calendar.getProperty("capabilities." + aCapability + ".supported") == false)
+    var value = (aCapability && !isSupported ? null : aValue);

But capSupported() checks for "!== false" and not "== false":

mozilla/calendar/prototypes/wcap/sun-calendar-event-dialog.js:
function capSupported(aCap) {
  var calendar = document.getElementById("item-calendar")
                         .selectedItem.calendar;
  return calendar.getProperty("capabilities." + aCap + ".supported") !== false;
}

Comment 6

[Berend Cornelius]

Attachment: patch v. 1

Sorry, I messed up "SetItemProperty" when shifting it to CalUtils.js as I had to adapt it slightly

Comment 8

[Philipp Kewisch [:Fallen] [:📆]]

Comment on attachment 302551 [details] [diff] [review]
patch v. 1


>-    var isSupported = (item.calendar.getProperty("capabilities." + aCapability + ".supported") == false)
>+    var isSupported = (item.calendar.getProperty("capabilities." + aCapability + ".supported") != false)
Not quite, you need the more strict !== here.

I'd prefer you move capSupported into calUtils.js though.

r+ with either of those things changed.

Comment 9

[Berend Cornelius]

I added the hint of Philipp. "capSupported" was not usable for calUtils as it referred to the dialog directly.

patch checked in on trunk and MOZILLA_1_8_BRANCH

=> FIXED

Comment 11

[Andreas Treumann]

Checked in Lightning and Sunbird build 20080211 -> task is fixed and verified.

Comment 12

[Lars Wohlfahrt (thetux)]

VERIFIED

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.1.13pre) Gecko/20080211 Calendar/0.8pre