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

Status: RESOLVED FIXED

(Toolkit Graveyard :: Microformats, defect)

Description

[Mike Kaply [:mkaply]]

Attachment: 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.

Comment 1

[Mike Kaply [:mkaply]]

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!

Comment 2

[Mike Kaply [:mkaply]]

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.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

./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']

Comment 4

[Mike Kaply [:mkaply]]

Attachment: 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.

Comment 5

[Mike Kaply [:mkaply]]

Fixed