Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom namespaces #45

Merged
merged 1 commit into from
Jun 17, 2014
Merged

Custom namespaces #45

merged 1 commit into from
Jun 17, 2014

Conversation

goetas
Copy link
Member

@goetas goetas commented Jun 9, 2014

An extended implementation of #44 that supports also custom namespace prefixes and XML style namespaces.

See here to see an example: https://github.com/goetas/html5-php/blob/custom-namespaces/test/HTML5/Parser/DOMTreeBuilderTest.php#L99

@goetas
Copy link
Member Author

goetas commented Jun 9, 2014

@cognifloyd ping

@goetas goetas changed the title Custom namespaces [WIP] Custom namespaces Jun 9, 2014
@goetas
Copy link
Member Author

goetas commented Jun 9, 2014

Even if it is not standard, this solution can help us to deal with some edge cases.

$html = new HTML5(array(
  "implicitNamespaces"=>["t"=>"http://www.example.com"]
));

$dom = $html->loadHTML('<t:div/>');

$dom->documentElement->namespaceURI; // http://www.example.com

$xp = new DOMXPath($dom);
$xp->registerNamespace("x", "http://www.example.com");
$xp->query("//x:div")->length; // 1

Should work also:

<body xmlns:x="http://www.prefixed.com">
    <a id="bar1" xmlns="bar1">
        <b id="bar4" xmlns="bar4"><x:prefixed id="prefixed"/></b>
    </a>
    <svg id="svg"></svg>
    <c id="bar2" xmlns="bar2"></c>
    <d id="bar3"></d>
</body>
$bar1  = $dom->getElementById('bar1');
$this->assertEquals('bar1', $bar1->namespaceURI);

$bar2  = $dom->getElementById('bar2');
$this->assertEquals("bar2", $bar2->namespaceURI);

$bar3  = $dom->getElementById('bar3');
$this->assertEquals("http://www.w3.org/1999/xhtml", $bar3->namespaceURI);

$bar4  = $dom->getElementById('bar4');
$this->assertEquals("bar4", $bar4->namespaceURI);

$svg  = $dom->getElementById('svg');
$this->assertEquals("http://www.w3.org/2000/svg", $svg->namespaceURI);

$svg  = $dom->getElementById('prefixed');
$this->assertEquals("http://www.prefixed.com", $svg->namespaceURI);

@goetas
Copy link
Member Author

goetas commented Jun 9, 2014

What about this problem? https://github.com/goetas/html5-php/pull/1

$svg = $dom->getElementById('svg');
$this->assertEquals("http://www.w3.org/2000/svg", $svg->namespaceURI);

$svg = $dom->getElementById('prefixed');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you meant $prefixed...

@cognifloyd
Copy link
Contributor

Thanks @goetas this is much cleaner than my quick hack.

The implicitNamespaces and xmlNamespaces options will need to be mentioned in the readme.

+1 from me. Now, I'll look at goetas#1

@goetas
Copy link
Member Author

goetas commented Jun 9, 2014

@mattfarina @technosophos What is your about this implementation?

@technosophos
Copy link
Member

I've been following this conversation, and I think this is the right direction for adding namespace support. We should probably have tests for HTML5 serialization to make sure that the result is valid or at least well formed enough that the serializer doesn't need any special case logic.

Overall, though, I am excited that you all have made HTML5 into something far more flexible than what we initially envisioned.

@goetas
Copy link
Member Author

goetas commented Jun 9, 2014

@technosophos i have wrote this test case to test some serialization isses https://github.com/goetas/html5-php/pull/1

It is not a clean approach but it works...

@goetas goetas mentioned this pull request Jun 9, 2014
@goetas
Copy link
Member Author

goetas commented Jun 9, 2014

@technosophos the approval of #37 is fundamental...

@goetas goetas added this to the 2.0 milestone Jun 10, 2014
@goetas
Copy link
Member Author

goetas commented Jun 11, 2014

@technosophos 2.0 will not be compatible with QueryPath... (css expressions now require a xml namespace...)

div#foo => //x:div[@id='foo'] and x has to be binded to http://www.w3.org/1999/xhtml/

@technosophos
Copy link
Member

I thought I had a workaround for that in QueryPath already. It's not all that uncommon to get namespaced documents in QueryPath.

But if that breaks QueryPath, so be it. I'll just have to go fix QueryPath.

(It can handle namespaces with the CSS3 syntax: x|div#foo, but almost nobody knows about that syntax, so I'll have to come up with a more elegant workaround.)

@goetas
Copy link
Member Author

goetas commented Jun 12, 2014

Almost ready

@goetas goetas mentioned this pull request Jun 12, 2014
@@ -187,6 +186,9 @@ protected function attrs($ele)
// prefix. It seems that DOM does this for us already, but there
// may be exceptions.
$name = $node->name;
if ($name == "xmlns:x___xmlns__x") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be documented somewhere, so that if someone uses the parser with their own serializer, they can still handle this case. Maybe this could go in the readme section about "known issues".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A PR is welcome 😄 (implicitNamespaces and xmlNamespaces option too)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add it to my list of stuff to document: cognifloyd#3

@goetas goetas changed the title [WIP] Custom namespaces Custom namespaces Jun 17, 2014
@goetas goetas merged commit 44f07f1 into Masterminds:master Jun 17, 2014
goetas added a commit that referenced this pull request Jun 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants