Skip to content

Commit

Permalink
DOMCharacterData::$data and DOMAttr::$value are writable
Browse files Browse the repository at this point in the history
  • Loading branch information
@kocsismate
kocsismate committed Aug 25, 2021
1 parent 2dafb0e commit cdf2f3e
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 3 deletions.
2 changes: 0 additions & 2 deletions ext/dom/php_dom.stub.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,6 @@ public function item(int $index) {}

class DOMCharacterData extends DOMNode implements DOMChildNode
{
/** @readonly */
public string $data;

/** @readonly */
Expand Down Expand Up @@ -285,7 +284,6 @@ class DOMAttr extends DOMNode
/** @readonly */
public bool $specified = true;

/** @readonly */
public string $value;

/** @readonly */
Expand Down
2 changes: 1 addition & 1 deletion ext/dom/php_dom_arginfo.h
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* This is a generated file, edit the .stub.php file instead.
* Stub hash: cff5c4b824da940f151617d08bb6b2419a9d26e9 */
* Stub hash: a01ea3d7803ba6099c79365ab8f45ca8d4f5d447 */

ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(arginfo_dom_import_simplexml, 0, 1, DOMElement, 0)
ZEND_ARG_TYPE_INFO(0, node, IS_OBJECT, 0)
Expand Down

3 comments on commit cdf2f3e

@nikic
Copy link
Member

@nikic nikic commented on cdf2f3e Aug 25, 2021

Choose a reason for hiding this comment

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

When I looked into this, I found that the last three DOMEntity properties are also writable -- though they are dummy properties, they just always return null and writing them doesn't do anything.

Checking the DOM standard, I found that the DOM 3 working draft had these properties as writable, while the final version has them as readonly -- but also uses different names for them. In DOM 4 they are entirely gone.

So I think it should be fine to make them actually readonly in the implementation at least.

@kocsismate
Copy link
Member Author

Choose a reason for hiding this comment

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

So I think it should be fine to make them actually readonly in the implementation at least.

Thanks for checking this! I'll submit a PR later. :)

@kocsismate
Copy link
Member Author

Choose a reason for hiding this comment

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

For reference: I've just submitted #7406

Please sign in to comment.