Fix channels for the ics provider

RESOLVED FIXED in 1.0

Status

defect
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Fallen, Assigned: Fallen)

Tracking

unspecified
x86
macOS
Dependency tree / graph

Details

Attachments

(1 attachment)

Posted patch Fix - v1 β€” β€” Splinter Review
For the offline patch in bug 380060 we noticed that in certain cases the channel was not being passed on correctly.

This patch changes the channel handling a bit and also provides some more log messages as in bug 325519.
Attachment #565488 - Flags: review?(matthew.mecca)
Comment on attachment 565488 [details] [diff] [review]
Fix - v1

Review of attachment 565488 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, r=mmecca with a few small nits

::: calendar/providers/ics/calICSCalendar.js
@@ +425,4 @@
>      onDataAvailable: function(request, ctxt, inStream, sourceOffset, count) {
>           // All data must be consumed. For an upload channel, there is
>           // no meaningfull data. So it gets read and then ignored
> +         var scriptableInputStream =

var to let.

@@ +438,3 @@
>          try {
> +            httpChannel = request.QueryInterface(Components.interfaces.nsIHttpChannel);
> +            cal.LOG("[calICSCalendar] channel.requestSucceeded: " + channel.requestSucceeded);

We need to change channel to httpChannel here also.

@@ +576,4 @@
>              this.writeICS();
>          }
>          else if (refreshAction) {
> +            cal.LOG("[calICSCalendar] Forcing Refresh of " + this.name);

We're not necessarily doing a force refresh here (only when refreshAction.forceRefresh is true)

@@ +818,4 @@
>          } catch(e) {
>              // For local files, asyncOpen throws on new (calendar) files
>              // No problem, go and upload something
> +            cal.LOG("Backup failed in asyncOpen:"+e);

We might as well make this consistent and prepend "[calIcsCalendar]"
Attachment #565488 - Flags: review?(matthew.mecca) → review+
Pushed to comm-central <http://hg.mozilla.org/comm-central/rev/c32fffefbfd9>
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Trunk
Backported to comm-aurora <http://hg.mozilla.org/releases/comm-aurora/rev/12a69b142106>
Target Milestone: Trunk → 1.0b9
Backported to comm-beta <http://hg.mozilla.org/releases/comm-beta/rev/8b2383e6e813>
Target Milestone: 1.0b9 → 1.0b7
Target Milestone: 1.0b7 → 1.0b8
Nits fixed, thank you very much. Great catches there!
You need to log in before you can comment on or make changes to this bug.