319557 - Make exporting to html use hCalendar classes

Status: RESOLVED FIXED

(Calendar :: Internal Components, defect)

Description

[Michiel van Leeuwen (email: mvl+moz@)]

It would be nice if exporting to html added hCalendar classes.

Comment 1

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: patch v1

This patch makes it so. It also switches to output to use dl, dt and dd instead of messing with b tags.

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

I had a bunch more discussion with Hixie and Tantek, and it turns out that hCalendar can, in fact, be used with HTML.  I'll try and review tomorrow.

Comment 3

[Stefan Sitter [:ssitter]]

I think we should escape the text (title, location, ...) before writing to html to avoid any potential parser errors. With that I mean replacing certain characters like  
  & with &
  < with &lt;
  > with &gt;
  " with &quot;

Comment 4

[Stefan Sitter [:ssitter]]

I tried the patch and here are my additional comments:

1) missing ; in line 123: str += "</dl>\n"

2) export fails if no description is set in event:
     Error: desc has no properties
     Source File: file:///C:/sb/dist/bin/components/calHtmlExport.js
     Line: 107:   if (desc.length > 0) { 

3) The exported file specifies UTF-8 character encoding but is not UTF-8
   encoded. For example the character 'ö' is displayed as '?' in Firefox when
   selecting utf-8 encoding. When switching to a non unicode encoding like
   iso-8859-1 it is properly displayed as 'ö'.

4) My opinion: Using a definition list does not look good as output. The result
   looks something like:
Title
    Sample All Day Event
When
    2005/12/17 00:00:00 floating – 2005/12/18 00:00:00 floating 
Location
    Somewhere
Description
    Lorem ipsum dolor sit amet, consectetuer adipiscing elit.
    Curabitur ante ipsum, tristique non, aliquam eget, hendrerit eget, tortor.
Title
...

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

(In reply to comment #3)
> I think we should escape the text (title, location, ...) before writing to html
> to avoid any potential parser errors. With that I mean replacing certain
> characters like  
>   & with &
>   < with &lt;
>   > with &gt;
>   " with &quot;
> 

In fact, until we do this, we're creating HTML that is subject to cross-site scripting attacks. e.g. someone tricks a user into subscribing to and exporting a calendar which has hostile HTML / JS in it.

That said, I'd be ok with filing a separate bug on that, since that problem existed before and independently of this patch.

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

(In reply to comment #4)
> I tried the patch and here are my additional comments:
> 
> 1) missing ; in line 123: str += "</dl>\n"
> 
> 2) export fails if no description is set in event:
>      Error: desc has no properties
>      Source File: file:///C:/sb/dist/bin/components/calHtmlExport.js
>      Line: 107:   if (desc.length > 0) { 
> 
> 3) The exported file specifies UTF-8 character encoding but is not UTF-8
>    encoded. For example the character 'ö' is displayed as '?' in Firefox when
>    selecting utf-8 encoding. When switching to a non unicode encoding like
>    iso-8859-1 it is properly displayed as 'ö'.

The above do need to be addressed before the patch is ready for review, I think.

> 4) My opinion: Using a definition list does not look good as output. The result
>    looks something like:
> Title
>     Sample All Day Event
> When
>     2005/12/17 00:00:00 floating – 2005/12/18 00:00:00 floating 
> Location
>     Somewhere
> Description
>     Lorem ipsum dolor sit amet, consectetuer adipiscing elit.
>     Curabitur ante ipsum, tristique non, aliquam eget, hendrerit eget, tortor.
> Title
> ...

So your objection is that DL has default style rules in most browsers that you don't like?  If that is the consensus (I don't have any particular feeling about this), possible alternatives I can think of include:

* override those style rules in our HTML
* output DIVs with style rules

or was there something else you had in mind?

Comment 7

[Stefan Sitter [:ssitter]]

(In reply to comment #6)
> So your objection is that DL has default style rules in most browsers that you
> don't like?  If that is the consensus (I don't have any particular feeling
> about this), possible alternatives I can think of include:
> * override those style rules in our HTML

No objection, just my privat opinion. (There's no accounting for taste.)
Mvl already experimented with some styling rules that will make the output more presentable.

Comment 8

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: patch v2

This patch uses e4x, to reduce the need for escaping etc.
It also uses divs, instead of dl/dt/dd.
It has styles defined, to make the output looka  bit better.
And it uses calIDateTimeFormatter, for l10n friendlyness.

Comment 9

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: use utf8

Previous patch didn't properly convert to utf8, this patch does.
(The conversion is in the exporter, because not all exporters will want to output utf8. we can't force it on them)

Comment 10

[Michiel van Leeuwen (email: mvl+moz@)]

Attachment: example output

Comment 11

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 208487 [details] [diff] [review]
use utf8

This looks great!  A couple of minor nits:

>+        // Include the end date anyway, even when empty, because the dtend
>+        // class should be there, for hCalendar goodness.
>+        var joiner = "";
>+        if (endstr.value) {
>+            joiner = " - ";
>         }

"separator" seems like it would be a more meaningful variable name than "joiner".

Also, if you could put a comment somewhere in the generated HTML about the intent of the summarykey class, that'd be great.

r=dmose

Comment 12

[Michiel van Leeuwen (email: mvl+moz@)]

patch checked in