Bug 256536 – XPath: Apply ignore-case matching to attribute names

• Status: RESOLVED
• Resolution: FIXED
• Component: XML

[Ahmad Saleem]

Created attachment 466294 [details]
GitHub Patch Tried

Hi Team,

While going through Blink's commit, I came across another potential merge but I am getting some C++ errors, so not able to compile:

Blink Commit - https://chromium.googlesource.com/chromium/src.git/+/0292ac42f12b0d7725ed1a311cce46290217a3bb

WPT failing test - http://wpt.live/domxpath/002.html

WebKit Source - https://searchfox.org/wubkat/source/Source/WebCore/xml/XPathStep.cpp#189 etc.

___________

Attached screenshots does not work at Line 354 where (as per 1-1 of Blink commit):

auto attr;

^ Does not work.

_________

Appreciate if someone can guide or if someone can fix this.

Thanks!

[Ahmad Saleem]

This is error, I get:

declaration of variable 'attr' with deduced type 'auto *' requires an
      initializer

[Chris Dumez]

(In reply to Ahmad Saleem from comment #1)
> This is error, I get:
> 
> declaration of variable 'attr' with deduced type 'auto *' requires an
>       initializer

Probably want to use something like:
```
RefPtr<Attr> attr;

// We need this branch because getAttributeNodeNS() doesn't do
// ignore-case matching even for an HTML element in an HTML document.
if (m_nodeTest.m_namespaceURI.isNull())
    attr = contextElement->getAttributeNode(m_nodeTest.m_data);
else
    attr = contextElement->getAttributeNodeNS(m_nodeTest.m_namespaceURI, m_nodeTest.m_data);
}
```

[Ahmad Saleem]

(In reply to Chris Dumez from comment #2)
> (In reply to Ahmad Saleem from comment #1)
> > This is error, I get:
> > 
> > declaration of variable 'attr' with deduced type 'auto *' requires an
> >       initializer
> 
> Probably want to use something like:
> ```
> RefPtr<Attr> attr;
> 
> // We need this branch because getAttributeNodeNS() doesn't do
> // ignore-case matching even for an HTML element in an HTML document.
> if (m_nodeTest.m_namespaceURI.isNull())
>     attr = contextElement->getAttributeNode(m_nodeTest.m_data);
> else
>     attr = contextElement->getAttributeNodeNS(m_nodeTest.m_namespaceURI,
> m_nodeTest.m_data);
> }
> ```

Let me try it in local build, if it works. Will do PR. Thanks for your help.

[Ahmad Saleem]

Created attachment 466326 [details]
Compiling version

^ This attached compiles. @Chris - thanks for your help.

Although, there is no progression on WPT. Which is weird. :-(

[Chris Dumez]

(In reply to Ahmad Saleem from comment #4)
> Created attachment 466326 [details]
> Compiling version
> 
> ^ This attached compiles. @Chris - thanks for your help.
> 
> Although, there is no progression on WPT. Which is weird. :-(

Can you try this for the top part of the change?
```
                auto& attr = downcast<Attr>(node);
                if (attr.document().isHTMLDocument() && attr.ownerElement() &&
                    attr.ownerElement()->isHTMLElement() && namespaceURI.isNull() &&
                    attr.namespaceURI().isNull()) {
                    return equalIgnoringASCIICase(attr.localName(), name);
                }
```

I think you may have converted the other part wrong from Blink also.

[Ahmad Saleem]

Created attachment 466328 [details]
Update version

(In reply to Chris Dumez from comment #5)
> (In reply to Ahmad Saleem from comment #4)
> > Created attachment 466326 [details]
> > Compiling version
> > 
> > ^ This attached compiles. @Chris - thanks for your help.
> > 
> > Although, there is no progression on WPT. Which is weird. :-(
> 
> Can you try this for the top part of the change?
> ```
>                 auto& attr = downcast<Attr>(node);
>                 if (attr.document().isHTMLDocument() && attr.ownerElement()
> &&
>                     attr.ownerElement()->isHTMLElement() &&
> namespaceURI.isNull() &&
>                     attr.namespaceURI().isNull()) {
>                     return equalIgnoringASCIICase(attr.localName(), name);
>                 }
> ```
> 
> I think you may have converted the other part wrong from Blink also.


I tried this as well but no progression.

[Chris Dumez]

Pull request: https://github.com/WebKit/WebKit/pull/13806

[Chris Dumez]

(In reply to Ahmad Saleem from comment #6)
> Created attachment 466328 [details]
> Update version
> 
> (In reply to Chris Dumez from comment #5)
> > (In reply to Ahmad Saleem from comment #4)
> > > Created attachment 466326 [details]
> > > Compiling version
> > > 
> > > ^ This attached compiles. @Chris - thanks for your help.
> > > 
> > > Although, there is no progression on WPT. Which is weird. :-(
> > 
> > Can you try this for the top part of the change?
> > ```
> >                 auto& attr = downcast<Attr>(node);
> >                 if (attr.document().isHTMLDocument() && attr.ownerElement()
> > &&
> >                     attr.ownerElement()->isHTMLElement() &&
> > namespaceURI.isNull() &&
> >                     attr.namespaceURI().isNull()) {
> >                     return equalIgnoringASCIICase(attr.localName(), name);
> >                 }
> > ```
> > 
> > I think you may have converted the other part wrong from Blink also.
> 
> 
> I tried this as well but no progression.

Well, I used the code I pasted above and got a progression so I uploaded a PR. Not sure why you saw different results.

[Ahmad Saleem]

(In reply to Chris Dumez from comment #8)
> (In reply to Ahmad Saleem from comment #6)
> > Created attachment 466328 [details]
> > Update version
> > 
> > (In reply to Chris Dumez from comment #5)
> > > (In reply to Ahmad Saleem from comment #4)
> > > > Created attachment 466326 [details]
> > > > Compiling version
> > > > 
> > > > ^ This attached compiles. @Chris - thanks for your help.
> > > > 
> > > > Although, there is no progression on WPT. Which is weird. :-(
> > > 
> > > Can you try this for the top part of the change?
> > > ```
> > >                 auto& attr = downcast<Attr>(node);
> > >                 if (attr.document().isHTMLDocument() && attr.ownerElement()
> > > &&
> > >                     attr.ownerElement()->isHTMLElement() &&
> > > namespaceURI.isNull() &&
> > >                     attr.namespaceURI().isNull()) {
> > >                     return equalIgnoringASCIICase(attr.localName(), name);
> > >                 }
> > > ```
> > > 
> > > I think you may have converted the other part wrong from Blink also.
> > 
> > 
> > I tried this as well but no progression.
> 
> Well, I used the code I pasted above and got a progression so I uploaded a
> PR. Not sure why you saw different results.

Win is a win.. More failing cases fixed on WPT.. :-)

[EWS]

Committed 264030@main (0386e818321f): <https://commits.webkit.org/264030@main>

Reviewed commits have been landed. Closing PR #13806 and removing active labels.

[Radar WebKit Bug Importer]

<rdar://problem/109283215>