Closed Bug 429181 Opened 12 years ago Closed 12 years ago

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

Categories

(Toolkit Graveyard :: Microformats, defect, minor)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED
mozilla1.9

People

(Reporter: karns.17, Assigned: mkaply)

Details

Attachments

(3 files, 1 obsolete file)

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.
Can you please post a link or an actual testcase?

Thanks.
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).
Attached patch Fix for problem (obsolete) — Splinter Review
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.
Attachment #316097 - Flags: review?(sayrer)
Will that also follow any 'includes' that are in the headers? (Similar to a nested include.)
(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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
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)
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
Component: Operator → Microformats
OS: Windows XP → All
Product: Mozilla Labs → Toolkit
QA Contact: operator → microformats
Hardware: PC → All
Target Milestone: --- → mozilla1.9
Version: unspecified → Trunk
Attached patch Better fixSplinter Review
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.
Attachment #316097 - Attachment is obsolete: true
Attachment #317596 - Flags: review?(sayrer)
Attachment #316097 - Flags: review?(sayrer)
Attached file 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
Attachment #317596 - Flags: review?(sayrer) → review+
Attachment #317601 - Attachment mime type: application/octet-stream → text/plain
fixed in hg
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment on attachment 317596 [details] [diff] [review]
Better fix

Microformats correctness. Would be nice to have in 3.0
Attachment #317596 - Flags: approval1.9.0.3?
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.
Attachment #317596 - Flags: approval1.9.0.4? → approval1.9.0.4-
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.
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.