429181 - @headers attribute on TD not followed when include-pattern is used

Status: RESOLVED FIXED

(Toolkit Graveyard :: Microformats, defect)

Description

[Jason]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5) Gecko/2008032620 Firefox/3.0b5
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5) Gecko/2008032620 Firefox/3.0b5

I was marking up an hCalendar in a <table> and Operator was nicely picking up additional properties using the @headers attributes on TDs.  Once I used the include-pattern to link from the vevent to an 'attendee' hcard, the properties specified in the 'headers' were no longer found.

Example:
<th id="ses1" axis="time" scope="row"><span class="summary">Session 1</span><br/><abbr class="dtstart" title="2008-04-19 09:00:00-0400">9:00</abbr>-<abbr class="dtend" title="2008-04-19 10:10:00-0400">10:10 AM</abbr></th>
<td headers="ses1" class="vevent"><a href="#WPF_for_Developers">WPF for Developers</a> (<a class="speaker" href="#JS">John Smith</a>)</td>

The vevent in the TD above picked up the summary, dtstart, and dtend from the TH specified in the TD@headers (ID=ses1).

I then changed the TD to use the include-pattern to link to John Smith's hCard as an attendee, and the summary, dtstart and dtend were no longer found.

<td headers="ses1" class="vevent"><a href="#WPF_for_Developers">WPF for Developers</a> (<a class="include speaker" href="#JS">John Smith</a>)</td>

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[Mike Kaply [:mkaply]]

Can you please post a link or an actual testcase?

Thanks.

Comment 2

[Jason]

I've added an html page with a matrix of test cases. One of the test case sets is using the include-pattern from a TH cell referenced by a TD cell.  I haven't seen any documentation whether those type of include links should be followed, but my intuition is that the '@headers pattern' should be treated like the include-pattern and allow nested include-patterns (so long as they are not referencing ancestors).

Comment 3

[Jason]

Attachment: Test Cases using hCalendar in a table with @headers

Comment 4

[Mike Kaply [:mkaply]]

Attachment: Fix for problem

OK, I think I understand.

The problem was in my preprocessing I was using "either or" for headers and include.

Now I do both.

Note that while this is an Operator fix here, I'm asking for review so it can go in the Firefox microformats code as well.

Comment 5

[Jason]

Will that also follow any 'includes' that are in the headers? (Similar to a nested include.)

Comment 6

[Mike Kaply [:mkaply]]

(In reply to comment #5)
> Will that also follow any 'includes' that are in the headers? (Similar to a
> nested include.)
> 

No it will not. Nested includes opens up a large can of worms so I have avoided it, especially since I have seen no examples of it in the wild.

Comment 7

[Jason]

I thought it was a (not completely) simple case of only following include links that do not reference ancestors? As for having it in the wild, my website (3amproductions.net) utilized nested includes at one point until I discovered so little support for them.  It seems to be a little backwards to not support a feature because it hasn't been found in use, when it's most likely not in use because no tool supports it. (A nice catch-22)

Comment 8

[Mike Kaply [:mkaply]]

The problem with nested includes is having to recursively call yourself in the code.

And honestly, I'm not convinced of the use case. 

Even includes/headers themselves is not a common use case, so nesting them seems even less common.

Can you put up a test page that looked like your original site?

Thanks

Comment 9

[Mike Kaply [:mkaply]]

Attachment: Better fix

This is a better fix that handles headers and includes separately.

Headers need to be done separately from includes, and they need to be done first.

(headers can reference includes, includes can't reference headers)

I'm working on a good unit test for this.

Comment 10

[Mike Kaply [:mkaply]]

Attachment: Unit test

Here's the unit test. It has a header that references two different includes and one of those includes references another include.

So it's testing header -> includes and header -> include -> include

Comment 11

[Mike Kaply [:mkaply]]

fixed in hg

Comment 12

[Mike Kaply [:mkaply]]

Comment on attachment 317596 [details] [diff] [review]
Better fix

Microformats correctness. Would be nice to have in 3.0

Comment 13

[Mike Connor [:mconnor]]

Comment on attachment 317596 [details] [diff] [review]
Better fix

In order to reduce risk and minimize overhead for stability releases, we are not accepting "nice to have" patches in stability releases.  As 3.1 is fast approaching, this should not unduly impact time to get these fixes to users.

Comment 14

[Mike Connor [:mconnor]]

Comment on attachment 317596 [details] [diff] [review]
Better fix

In order to reduce risk and minimize overhead for stability releases, we are not accepting "nice to have" patches in stability releases.  As 3.1 is fast approaching, this should not unduly impact time to get these fixes to users.