Filter: Create wrapper functions to load/serialize a DOM. [#542742]

#374441: Refactor Drupal HTML corrector (PHP5) added some code to load a text into a DOM object and then convert that back to a string.
As proved by #362972: Filters should remove "rel" attributes instead of just adding rel="nofollow" and #16161: Move Read More link to end of node content the exactly same code will be used by any filter that doesn't want to use regexp voodoo to alter the HTML.
Given that here is a patch to create 2 functions used to load and serialize the DOM. It probably needs phpdocs improvements.

Comment	File	Size	Author
#9	542742-9-create-dom-load-serialize-functions.patch	2.95 KB	tic2000
#9
#7	542742-7-create-dom-load-serialize-functions.patch	2.98 KB	tic2000
#7
#6	542742-6-create-dom-load-serialize-functions.patch	2.97 KB	tic2000
#6
#5	542742-5-create-dom-load-serialize-functions.patch	2.93 KB	tic2000
#5
#4	542742-4-create-dom-load-serialize-functions.patch	2.26 KB	tic2000
#4
#2	542742-2-create-dom-load-serialize-functions.patch	2.24 KB	tic2000
#2
#1	542742-create-dom-load-serialize-functions.patch	2.57 KB	tic2000
#1
	create-dom-load-serialize-functions.patch	2.59 KB	tic2000

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

tic2000 CreditAttribution: tic2000 commented 7 August 2009 at 12:36

File	Size
542742-create-dom-load-serialize-functions.patch	2.57 KB

And a patch with a one line html corrector.

Comment #2

tic2000 CreditAttribution: tic2000 commented 7 August 2009 at 17:43

File	Size
542742-2-create-dom-load-serialize-functions.patch	2.24 KB

And now a patch that removes the first regexp.
I hope that not many kittens are killed in the process.

Comment #3

7 August 2009 at 18:00

Status:

Needs review

» Needs work

The last submitted patch failed testing.

Comment #4

tic2000 CreditAttribution: tic2000 commented 7 August 2009 at 18:12

Status:

Needs work

» Needs review

File	Size
542742-4-create-dom-load-serialize-functions.patch	2.26 KB

I saw that coming. New patch. It should fix the exceptions.

Comment #5

tic2000 CreditAttribution: tic2000 commented 8 August 2009 at 14:34

File	Size
542742-5-create-dom-load-serialize-functions.patch	2.93 KB

Comment #6

tic2000 CreditAttribution: tic2000 commented 8 August 2009 at 15:21

File	Size
542742-6-create-dom-load-serialize-functions.patch	2.97 KB

Comment #7

tic2000 CreditAttribution: tic2000 commented 8 August 2009 at 19:54

File	Size
542742-7-create-dom-load-serialize-functions.patch	2.98 KB

Comment #8

Damien Tournoud CreditAttribution: Damien Tournoud commented 11 August 2009 at 08:01

Thanks, tic2000.

A few nitpicks:

+ * You can use filter_dom_serialize( ) to serialize this DOMDocument

^ That should be filter_dom_serialize()

+ * The function serializes the body part of a DOMDocument
+ * back to an XHTML snippet.
+ * The resulting XHTML snippet will have a space before the trailing />
+ * of closing elements, for better rendering on HTML user agents.

There is a blank line missing between the two sentences.

Let's rewrite the second one as "The resulting XHTML snippet will be properly formatted to be compatible with HTML user agents."

Comment #9

tic2000 CreditAttribution: tic2000 commented 11 August 2009 at 09:47

File	Size
542742-9-create-dom-load-serialize-functions.patch	2.95 KB

Comment #10

Damien Tournoud CreditAttribution: Damien Tournoud commented 11 August 2009 at 09:48

Status:

Needs review

» Reviewed & tested by the community

Yay! Thanks.

Comment #11

Dries CreditAttribution: Dries commented 11 August 2009 at 12:01

Status:

Reviewed & tested by the community

» Needs work

Committed this to CVS HEAD. Thanks!

I'm marking this 'code needs work' because I feel it is lacking some isolated tests -- or at least, maybe we should consider refactoring some of the existing tests. If you disagree, feel free to mark this 'fixed'.

Comment #12

tic2000 CreditAttribution: tic2000 commented 11 August 2009 at 13:37

What should they test? They are just some wrapper function. I mean the load wrapper just does DOMDocument::LoadHTML. I don't think we need a test for that.
The serialize is a little more complex, but I think that tests for HTML corrector cover that.