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.

Sign up for GitHub

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

Jump to bottom

[RFC] DOM HTML5 parsing and serialization support #12111

Merged
merged 53 commits into from Nov 13, 2023
Merged

[RFC] DOM HTML5 parsing and serialization support #12111

merged 53 commits into from Nov 13, 2023

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Sep 2, 2023

@TimWolla TimWolla added the RFC label Sep 2, 2023
@nielsdos nielsdos force-pushed the dom-bridge-lexbor branch 8 times, most recently from 53b3bfc to 1c75ea8 Compare September 21, 2023 19:52
@nielsdos nielsdos force-pushed the dom-bridge-lexbor branch 3 times, most recently from 16e8d23 to d5852be Compare September 26, 2023 20:19
@lexborisov
Copy link

@nielsdos

Is there anything I can do to help you?
For example, I could put Lexbor HTML parser (code exclusively for html parsing) in a separate repository.

@nielsdos
Copy link
Member Author

@nielsdos

Is there anything I can do to help you? For example, I could put Lexbor HTML parser (code exclusively for html parsing) in a separate repository.

Hi Alexander

I don't think a separate repository is necessary. To sync upstream changes into PHP I just copy the relevant folders. So it doesn't really matter if it's in a separate repository or not. Then I apply some patches on top.
The only more "complex" change is that I patch out calls to the CSS library. That's because we don't need it and we'd like to save a bit on the binary size. I think however that's difficult to change on Lexbor's side because the dependency there makes sense.

@lexborisov
Copy link

@nielsdos

Yes, CSS makes a huge difference to a project.
I think this can be used as a positive thing. There are CSS selectors in there - it's very handy for searching for elements in the HTML tree.

Nice! Feel free to ask any related questions.

@nielsdos
Copy link
Member Author

@nielsdos

Yes, CSS makes a huge difference to a project. I think this can be used as a positive thing. There are CSS selectors in there - it's very handy for searching for elements in the HTML tree.

I agree. Having CSS Selectors is something on my roadmap, but can only land after this has been accepted.

Nice! Feel free to ask any related questions.

Thanks for the offer :)

@nielsdos nielsdos force-pushed the dom-bridge-lexbor branch 4 times, most recently from 23229eb to 73ca97f Compare October 5, 2023 21:04
@nielsdos nielsdos force-pushed the dom-bridge-lexbor branch 3 times, most recently from 219f8bf to e43f2af Compare October 8, 2023 17:35
nielsdos and others added 23 commits November 13, 2023 19:24
@nielsdos
Propagate last error back into libxml
a30e5a6
@nielsdos
Propagate file name in libxml error
d161e54
@nielsdos
Update doctype hint in ext/xsl
23612b0
@nielsdos
Update error message wording of abstract class
1410238
@nielsdos @TimWolla
Add test for incompatible override_encoding and charset
44b9663 
Co-authored-by: Tim Düsterhus <timwolla@php.net>
@nielsdos
Test behaviour of XML-style namespaces in HTMLDocument
0b10fdc
@nielsdos
Process review feedback
965732d
@nielsdos
Use canonical names in the return types and argument types
bf89e80
@nielsdos
Use consistently camelCase for overrideEncoding
7ba177e
@nielsdos
Split long lines in html_document
3dbbf19
@nielsdos
Give magic values descriptive names
f798f45
@nielsdos
Codestyle nit
cf07ba9
@nielsdos
Reduce repetition
ea7d604
@nielsdos
Apply strict properties
d906c0d
@nielsdos
Code style nits
92e9064
@nielsdos
Review remarks
92faa43
@nielsdos
Code style linebreaks
c645f5d
@nielsdos
Make encoding explicit in XMLDocument
308313c
@nielsdos
Mark XMLDocument::validate() no longer as tentative-return-type
b7f065f
@nielsdos
Only explicitly set encoding if not provided by the XML parser
dbc9afa
@nielsdos
Code review fixes
c7b55c2
@nielsdos
Defensively set stream to NULL
557aa43
@nielsdos
NEWS
46db0d2
@nielsdos
Copy link
Member Author

@theseer No problem. Feel free to take a look later when you have more time.

@nielsdos nielsdos merged commit 1492be5 into php:master Nov 13, 2023
8 of 9 checks passed
@fxzxmicah
Copy link

After 9 years, this finally arrived.

This was referenced Jan 26, 2024
Closed
Merged
nicolas-grekas added a commit to symfony/symfony that referenced this pull request Jan 29, 2024
@nicolas-grekas
bug #53656 [Form] Use self-closing <input /> syntax again, reverting …
7aab885 
…#47715 (mpdude)

This PR was merged into the 6.4 branch.

Discussion
----------

[Form] Use self-closing `<input />` syntax again, reverting #47715

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | Fix #53649
| License       | MIT

This PR reverts #47715 and puts the trailing slash back into `<input .../>` markup in the form themes.

The reasons are outlined in detail in #53649. Basically, the trailing slash in void elements like `<input>` is not required in HTML 5, yet it is perfectly valid and compliant with specs.

Writing markup that way is required to parse and process it with an XML parser, like `libxml2` underlying all of the PHP DOM, XML etc. extensions.

HTML 5 capable parser support has been added to PHP in php/php-src#12111 and will be available with PHP 8.4. So, we might want to remove the trailing slashes once Symfony requires at least PHP 8.4.

Commits
-------

8c92200 Revert #47715
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TimWolla TimWolla TimWolla left review comments

@Girgias Girgias Girgias left review comments

@kocsismate kocsismate Awaiting requested review from kocsismate kocsismate is a code owner

Assignees
No one assigned
Labels
Category: Build System Extension: dom Extension: libxml Extension: xsl RFC
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@nielsdos @lexborisov @TimWolla @theseer @Girgias @fxzxmicah