Microformats.getParent returns wrong node when two or more ancestors are of the same type of microformat

RESOLVED FIXED

Status

()

Toolkit
Microformats
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: mkaply, Assigned: mkaply)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

11 years ago
Created attachment 301721 [details] [diff] [review]
Fix for problem - get the [1] node from xpath

If you have a scenario like this:

  <div class="vcard" id="outer_vcard">
    <div class="vcard" id="middle_vcard">
      <div class="vcard" id="inner_vcard">
        <span class="fn">Inner User</span>
      </div>
      <span class="fn">Middle User</span>
    </div>
    <span class="fn">Outer User</span>
  </div>

And you ask the "inner_vcard" what its parent is, it will tell you "outer_vcard" instead of "middle_vcard"

Fix is to be more explicit in what node I get from the XPath

testcase is attached as well.

Note testcase has some rework of the original microformats testcase to make it easier to add new tests.
Attachment #301721 - Flags: review?(sayrer)
(Assignee)

Comment 1

11 years ago
This isn't right either. There still seem to be cases where I get a different ancestor.

Jonas:

What is the correct xpath for "get my immediate ancestor that has a certain classname?

Thanks!
(Assignee)

Comment 2

11 years ago
Comment on attachment 301721 [details] [diff] [review]
Fix for problem - get the [1] node from xpath

Actually I'm doing this wrong.

Because I'm enumerating through the list first, I find the first parent that corresponds to the earliest microformat.

What I need to do is find the first parent that has any of my microformat class names. Solution coming.
Attachment #301721 - Flags: review?(sayrer)
Attachment #301721 - Attachment is patch: true
Attachment #301721 - Attachment mime type: application/octet-stream → text/plain
./ancestor::*[class='foo'][1]

Should return the innermost ancestor with a given class. So in the example above

./ancestor::*[class='vcard']

should yield the 'inner_vcard' node if the context node is the span.

If you want "the innermost ancestor but only if it has a certain class" the expression would be

./ancestor::*[1][class='vcard']
(Assignee)

Comment 4

11 years ago
Created attachment 301797 [details] [diff] [review]
Rewrite of getparent

This is a rewrite of getparent to construct one xpath to do what we need.

It also has full unit test with all permutations.
Attachment #301721 - Attachment is obsolete: true
Attachment #301797 - Flags: review?(sayrer)
(Assignee)

Updated

11 years ago
Blocks: 416134

Updated

11 years ago
Attachment #301797 - Flags: review?(sayrer) → review+
(Assignee)

Updated

11 years ago
Attachment #301797 - Flags: approval1.9?

Updated

11 years ago
Attachment #301797 - Flags: approval1.9? → approval1.9+
(Assignee)

Comment 5

11 years ago
Fixed
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.