1.01 KB, text/plain
ask correct REPORT request to the server according to http://tools.ietf.org/html/rfc4791#section-7.8.9
1.29 KB, patch
|Details | Diff | Splinter Review|
User-Agent: Mozilla/5.0 (X11; U; Linux i686; fr; rv:18.104.22.168pre) Gecko/20061023 SUSE/22.214.171.124-0.1 Firefox/126.96.36.199pre Build Identifier: lightning 2007050103 for linux and thunderbird 2 the checkbox in the todo tab "Show completed tasks" do not have any consequences when it checked or not. The bug is caused by the fact that ITEM_FILTER_COMPLETED_YES is not taken into account when sending report Reproducible: Always Steps to Reproduce: 1.select todo tab 2.uncheck the box 3. Actual Results: all the todo completed are visible Expected Results: should return only the pending To-Dos
Created attachment 263942 [details] [diff] [review] a possible patch This patch respect the caldav spec see : http://tools.ietf.org/html/rfc4791#section-7.8.9 The problem with this patch is that if the caldav server doesn't support prop-filter, is-not-defined, text-match then the UI won't show off any todo A better way, IMHO should have been to make apply the filter client side, but I'm not sure because if you want to use correctly CalDav capabilities, my patch is probably better Your view point ?
Summary: TODO list → "Show completed tasks" checkbox has no effect on CalDav calendar
First, thanks for the patch! Your approach looks sound to me at first glance, but I'm not going to give it a thorough review right now, because: * you haven't asked :-) * there's a patch already reviewed and waiting to be applied (after 0.5) in bug 378588 which will change this area of code. It probably would be easier for all concerned if you marked this bug as depending on 378588 and wrote the patch against a tree with that patch installed. Apologies if you already know this, but the patch should be against a Mozilla source tree (see http://developer.mozilla.org/en/docs/Creating_a_patch ) rather than against an installed Sunbird like this one is. Since it's a CalDAV patch, I'm probably the best person from whom to request review. * As you point out, this patch depends on the server implementing particular parts of the CalDAV spec. It seems to work fine with the Apple CalDAV server, and with RSCDS, but in my testing it causes a regression against the current version of Cosmo (Cosmo can't parse the query and just returns events, which then show up in the Sunbird tasklist). That's not necessarily a showstopper, but it's good to mention such things along with a patch, if possible. If you're interested in Sunbird/CalDAV development and don't have access to these different server types, I may be able to help. While it would be possible to apply the filter client-side, I think the correct approach is to insist that CalDAV servers follow the spec, as your patch does. I'll file a bug against Cosmo (which at any rate does not yet attempt to support CalDAV VTODOs and should be simply ignoring these queries) unless you'd prefer to do so yourself. Thanks again for the patch. I hope you will revise, resubmit, and request review.
(In reply to comment #2) First I'm sorry because I haven't seen bug 378588, even if I looked at similar bug before filling this one:-/ At the moment this patch won't work with RSCDS because it don't support at the prop-filter, is-not-defined, text-match. But I could implemented it in RSCDS, it shouldn't be that hard. I couldn't patch COSMO, I've never look at its code, and have no JAVA skills only PHP. I will look at your patch, and wait for it applied in the tree, then as you say > I think the correct > approach is to insist that CalDAV servers follow the spec, as your patch does. we should then try to implement more of the spec in caldav server.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Now that bug 378588 is fixed, does someone can review this small patch ? RSCDS (SVN version) support now the prop-filter, is-not-defined, text-match element, It should be interesting to have my patch REVIEW and if ok Checked in on HEAD and MOZILLA_1_8_BRANCH.
(In reply to comment #4) You need to request review from someone to get your patch reviewed. Check http://wiki.mozilla.org/Calendar:Module_Ownership to find the correct person to ask for review. But according to Comment #2 it seems that a new patch is required.
Following up on comment #5, just to make sure everything is clear: * you need to submit a new version of the patch since the current one will no longer apply cleanly * you need to request review using the form on the "Add an attachment" page, not via a bug comment. Set the review dropdown to ? and the Requestee to the appropriate module owner. In this case that's firstname.lastname@example.org; for other patches to other modules please consult the page Stefan cites in comment #5. * Thanks for the patch!
OK I will try tonight
Created attachment 273642 [details] [diff] [review] ask correct REPORT request to the server according to http://tools.ietf.org/html/rfc4791#section-7.8.9 I've tested this patch, by checking the query send to server, but unfortunately the server (RSCDS) doesn't work properly against this REPORT
Assignee: nobody → mdelorme
OS: Linux → All
Hardware: PC → All
Comment on attachment 273642 [details] [diff] [review] ask correct REPORT request to the server according to http://tools.ietf.org/html/rfc4791#section-7.8.9 r- for now based on comment #9
Attachment #273642 - Flags: review?(browning) → review-
patch works on current Apple CalendarServer if (and only if) the "and if not cancelled" part of the query is removed. I think we can live with this since our checkbox reads "Hide Completed Tasks", not "Hide Completed and Cancelled Tasks". RSCDS and Bedework both ignore the COMPLETED part of the query and return tasks regardless of the COMPLETED status. This at least allows users to use Tasks. Cosmo, unfortunately, still (0.6.1) returns events to the query and thus spams the task list into unusability. Bug filed at OSAF.
I am sorry but what I need to do for my patch being applied ?
Maxime, you should update your patch according comment#9 (last paragraph) and comment#12, attach it to this bug again and ask for review from Bruno Browning.
Created attachment 280798 [details] [diff] [review] ask correct REPORT request to the server according to http://tools.ietf.org/html/rfc4791#section-7.8.9 fix mis-aligned code, line should not so long, remove
Comment on attachment 280798 [details] [diff] [review] ask correct REPORT request to the server according to http://tools.ietf.org/html/rfc4791#section-7.8.9 looks good. This patch still causes "Hide Completed Tasks" to hide all tasks on RSCDS, but I think we can live with that and I understand it will be fixed soon with 0.8.1. Thanks for the patch! r=browning
Attachment #280798 - Flags: review?(browning) → review+
Does I need to do something for the patch to be applied ? Regards
Bruno, should this patch be checked in for 0.7 or post-0.7?
I've been intending to check it in all week but haven't found a moment. Will try to do so tomorrow morning - but if someone else can check it in first that's fine with me.
Checked in on HEAD and MOZILLA_1_8_BRANCH.
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.7
verified with Mozilla/5.0 (Windows; U; Windows NT 5.1; pl; rv:188.8.131.52pre) Gecko/20071002 Sunbird/0.7
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.