I'm not exactly sure why this is happening, but we've been getting php errors that look like this:
Error: Call to a member function getElementsByTagName() on null in /htdocs/modules/filter/filter.module, line 1123

Basically if you create a standard, empty DOMDocument object in PHP and then try to pass that to filter_dom_serialize() it will cause a white screen as it stands now.

$document = new DOMDocument();
$result = filter_dom_serialize($document);

this will cause drupal to throw the following error, usually on a white screen:

Error: Call to a member function getElementsByTagName() on null in /htdocs/modules/filter/filter.module, line 1123

CommentFileSizeAuthor
#50 error_call_to_a_member-2511306-d7-49.patch2.16 KBelusivemind
#47 error_call_to_a_member-2511306-d7-47.patch2.04 KBelusivemind
#44 error_call_to_a_member-2511306-d7-44.patch2.02 KBelusivemind
#38 error_call_to_a_member-2511306-38.patch1.98 KBelusivemind
#36 error_call_to_a_member-2511306-36.patch1.98 KBelusivemind
#29 error_call_to_a_member-2511306-29.patch1.86 KBelusivemind
#26 error_call_to_a_member-2511306-26-test-only.patch693 byteselusivemind
#26 error_call_to_a_member-2511306-26.patch1.18 KBelusivemind
#21 php-version.png337.26 KBelusivemind
#21 error-shown.png115.47 KBelusivemind
#20 error_call_to_a_member-2511306-20-test-only.patch742 byteselusivemind
#16 error_call_to_a_member-2511306-16.patch1.86 KBjeroent
#16 error_call_to_a_member-2511306-16-test-only.patch740 bytesjeroent
#11 core-fix-call-to-member-function-getelementsbytagname-error-d8.patch1.13 KBelusivemind
#5 core-fix-call-to-member-function-getelementsbytagname-error-d8.patch1.28 KBelusivemind
#1 core-fix-call-to-member-function-getelementsbytagname-error-2511306-0.patch1.25 KBleolandotan

Comments

leolandotan’s picture

leolandotan commented
Status: Active » Needs review
StatusFileSize
new core-fix-call-to-member-function-getelementsbytagname-error-2511306-0.patch1.25 KB

I created a patch from the mentioned link on the main issue description.

elusivemind’s picture

elusivemind commented

Good work on adding in this defensive programming. It has corrected an issue that reared its ugly head on an update to PHP 5.5.24 and then 5.5.25. Oddly, when you go to 5.5.26, it seems to work fine again - but still the patch you have submitted is a great addition and should be rolled ASAP into Drupal Core. That is my opinion after my review and implementation of your code on many sites including sites for

The Doors
Styx
Imagine Dragons
R5
and about 35 similar ones that were fixed with this patch.

Well done.

elusivemind’s picture

elusivemind commented
Status: Needs review » Reviewed & tested by the community
cilefen’s picture

cilefen commented
Status: Reviewed & tested by the community » Needs review

Please check if this is an issue with Drupal 8. If so, it must be fixed there first according to the backport policy.

elusivemind’s picture

elusivemind commented
StatusFileSize
new core-fix-call-to-member-function-getelementsbytagname-error-d8.patch1.28 KB

Confirmed the bug exists in D8. Patch attached

elusivemind’s picture

elusivemind commented
Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 5: core-fix-call-to-member-function-getelementsbytagname-error-d8.patch, failed testing.

cilefen’s picture

cilefen commented
Version: 7.38 » 8.0.x-dev
Status: Needs work » Needs review

Re #6, you cannot review your own patches.

cilefen queued 5: core-fix-call-to-member-function-getelementsbytagname-error-d8.patch for re-testing.

cilefen’s picture

cilefen commented 
+++ b/core/lib/Drupal/Component/Utility/Html.php
@@ -280,14 +280,18 @@ public static function serialize(\DOMDocument $document) {
+    // If we have invalid $document items, the $body_node will be empty and cause errors.
+    // This is defensive logic against that occurrance.

I am not sure these comments are necessary. The meaning of the code is clear to me.

elusivemind’s picture

elusivemind commented
StatusFileSize
new core-fix-call-to-member-function-getelementsbytagname-error-d8.patch1.13 KB

Comments concern addressed.

leolandotan’s picture

leolandotan commented

Thanks @ElusiveMind! :)

cilefen’s picture

cilefen commented

@leolando.tan Would you like to offer a review?

leolandotan’s picture

leolandotan commented
Status: Needs review » Reviewed & tested by the community

I have tested the patch by:

  1. updated drupal 8 copy using `git pull origin 8.0.x`
  2. install a fresh drupal site
  3. applied patch using `patch -p1 < core-fix-call-to-member-function-getelementsbytagname-error-d8.patch`

Since the Filter module's description is "Filters content in preparation for display.". The test contents I saved were blank, with HTML and "unclosed" HTML using basic, restricted and full mode and also in quick edit mode. So far checked the Status report and Recent log messages after accessing pages and adding dummy content, they appear clean with no errors.

There are no additions to the UI since this is a logical process fix.

@cilefen Sorry for responding quite late. I hope this review is alright.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

This needs test coverage.

Also if it really is only due to a regression in 5.5.25 we should document that.

jeroent’s picture

jeroent
Primary language Dutch
Location 🇧🇪
commented
Status: Needs work » Needs review
Issue tags: -Needs tests +DUGBE0609
StatusFileSize
new error_call_to_a_member-2511306-16-test-only.patch740 bytes
new error_call_to_a_member-2511306-16.patch1.86 KB

Created a test.

Patch error_call_to_a_member-2511306-16-test-only.patch should fail.

elusivemind’s picture

elusivemind commented

error_call_to_a_member-2511306-16-test-only.patch

Is a patch to Drupal 7. The original was an issue with Drupal 7, so I checked Drupal 8 for the same issue and patched it there.

cilefen’s picture

cilefen commented

Re #14 @leolando.tan That is great - it is actually much better than we usually get!

elusivemind’s picture

elusivemind commented

Re: #16 @JeroenT - I am wondering if this is because the test is running on something > PHP 5.5.25. I will do some work on this using the 5.5.25 environment and 5.5.24 and see if it does, indeed, fail there. As stated in #15, this really is an issue only with these two versions of PHP, but because Acquia just upgraded everyone to PHP 5.5.25, we are having this problem and we should have a defensively programmed solution. I will work on this today or tomorrow.

Thanks for the input. Will keep working.

elusivemind’s picture

elusivemind commented
StatusFileSize
new error_call_to_a_member-2511306-20-test-only.patch742 bytes

If passing only an instance of DOMDocument to Html::serialize, you will reproduce this error in Drupal 8. I have attached at test patch in an attempt to reproduce this error.

elusivemind’s picture

elusivemind commented
StatusFileSize
new error-shown.png115.47 KB
new php-version.png337.26 KB

I can only assume this passed because it is running a version of PHP that is not 5.5.24 or 5.5.25. I have attached the errors I am getting using the same code.

** edit ** - it marked it as succeeding at first

Status: Needs review » Needs work

The last submitted patch, 20: error_call_to_a_member-2511306-20-test-only.patch, failed testing.

elusivemind’s picture

elusivemind commented

Any chance I can get some traction on this since I produce a test that caused the problem?

elusivemind’s picture

elusivemind commented
Status: Needs work » Needs review
cilefen’s picture

cilefen commented 
+++ b/core/tests/Drupal/Tests/Component/Utility/HtmlTest.php
@@ -310,4 +310,16 @@ public function testDecodeEntitiesAndEscape() {
+    $document = new \DOMDocument();
+    //$document->loadHTML("<html>serialize test</html>");
+    $result = Html::serialize($document);

We can't have commented-out code committed.

We need a combined patch (#16 + #10) in addition to a test-only patch.

elusivemind’s picture

elusivemind commented
StatusFileSize
new error_call_to_a_member-2511306-26.patch1.18 KB
new error_call_to_a_member-2511306-26-test-only.patch693 bytes

Created a patch to address changes in #16 + 10. Also created a test only patch.

Status: Needs review » Needs work

The last submitted patch, 26: error_call_to_a_member-2511306-26-test-only.patch, failed testing.

The last submitted patch, 26: error_call_to_a_member-2511306-26-test-only.patch, failed testing.

elusivemind’s picture

elusivemind commented
StatusFileSize
new error_call_to_a_member-2511306-29.patch1.86 KB

Addressing testing issues. Test seems to fail if not included with main patch.

elusivemind’s picture

elusivemind commented
Status: Needs work » Needs review
cilefen’s picture

cilefen commented

Test seems to fail if not included with main patch.

That is a good thing.

elusivemind’s picture

elusivemind commented

Aye. Many apologies for the learning curve.

elusivemind’s picture

elusivemind commented

The patch for Drupal 8 has passed. What is the next step in getting this patch implemented for Drupal 8 as well as 7. I appreciate your patience and help.

elusivemind’s picture

elusivemind commented
Status: Needs review » Reviewed & tested by the community
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work 
  1. +++ b/core/lib/Drupal/Component/Utility/Html.php
@@ -279,15 +279,17 @@ public static function load($html) {
+    ¶
+    if ($body_node != NULL) {
+      foreach ($body_node->getElementsByTagName('script') as $node) {
+        static::escapeCdataElement($node);
+      }

    Trailing whitespace is fixable on commit.

    We should use !== for the $body_node != NULL comparison. 

  2. +++ b/core/tests/Drupal/Tests/Component/Utility/HtmlTest.php
@@ -310,4 +310,15 @@ public function testDecodeEntitiesAndEscape() {
+  public function testSerialize() {

    this could probably use some docs to explain what it's testing.

elusivemind’s picture

elusivemind commented
StatusFileSize
new error_call_to_a_member-2511306-36.patch1.98 KB

Added comments and removed trailing white spaces.

elusivemind’s picture

elusivemind commented
Status: Needs work » Needs review
elusivemind’s picture

elusivemind commented
StatusFileSize
new error_call_to_a_member-2511306-38.patch1.98 KB

Added type comparison (!==)

elusivemind’s picture

elusivemind commented
Status: Needs review » Reviewed & tested by the community
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.0.x, thanks!

  • catch committed 7d37585 on 8.0.x 
    Issue #2511306 by ElusiveMind, JeroenT, leolando.tan: Error: Call to a...
elusivemind’s picture

elusivemind commented

What about the version 7 patch?

cilefen’s picture

cilefen commented
Version: 8.0.x-dev » 7.x-dev
Status: Fixed » Patch (to be ported)
Issue tags: +Needs backport to D7

I moved it to D7 for the backport.

elusivemind’s picture

elusivemind commented
StatusFileSize
new error_call_to_a_member-2511306-d7-44.patch2.02 KB
elusivemind’s picture

elusivemind commented
Status: Patch (to be ported) » Needs review

Status: Needs review » Needs work

The last submitted patch, 44: error_call_to_a_member-2511306-d7-44.patch, failed testing.

elusivemind’s picture

elusivemind commented
StatusFileSize
new error_call_to_a_member-2511306-d7-47.patch2.04 KB

Added description to test

elusivemind’s picture

elusivemind commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 47: error_call_to_a_member-2511306-d7-47.patch, failed testing.

elusivemind’s picture

elusivemind commented
Status: Needs work » Needs review
StatusFileSize
new error_call_to_a_member-2511306-d7-49.patch2.16 KB
elusivemind’s picture

elusivemind commented
Status: Needs review » Reviewed & tested by the community

Ready for merge?

cilefen’s picture

cilefen commented
Status: Reviewed & tested by the community » Needs review

@ElusiveMind That's a +1 from you, but it's peer review to become RTBC.

cilefen’s picture

cilefen commented

Can you post the steps to reproduce in the issue summary? I'll try to get someone to review it in core mentoring.

elusivemind’s picture

elusivemind commented
Issue summary: View changes
charginghawk’s picture

charginghawk commented
Status: Needs review » Reviewed & tested by the community

I have tested the patch by:

- Cloning down D7

git clone --branch 7.x http://git.drupal.org/project/drupal.git

- Installing a fresh Drupal site

- Testing that it's broken

vagrant@vagrant-ubuntu-trusty-64:/vagrant/docroot$ drush eval '$document = new DOMDocument(); $result = filter_dom_serialize($document);'
PHP Fatal error:  Call to a member function getElementsByTagName() on a non-object in /vagrant/docroot/modules/filter/filter.module on line 1122
Drush command terminated abnormally due to an unrecoverable error.                        [error]
Error: Call to a member function getElementsByTagName() on a non-object in
/vagrant/docroot/modules/filter/filter.module, line 1122

- Applying patch

git apply -v error_call_to_a_member-2511306-d7-49.patch

- Testing it again

vagrant@vagrant-ubuntu-trusty-64:/vagrant/docroot$ drush eval '$document = new DOMDocument(); $result = filter_dom_serialize($document);'
vagrant@vagrant-ubuntu-trusty-64:/vagrant/docroot$

- No error!

I'd say RTBC.

David_Rothstein’s picture

David_Rothstein commented
Status: Reviewed & tested by the community » Fixed

Committed to 7.x - thanks!

Fixed on commit:

diff --git a/modules/filter/filter.test b/modules/filter/filter.test
index b8b911d..5471185 100644
--- a/modules/filter/filter.test
+++ b/modules/filter/filter.test
@@ -1985,7 +1985,7 @@ class FilterSettingsTestCase extends DrupalWebTestCase {
 }
 
 /**
- * Tests DOMDocument Serialization.
+ * Tests DOMDocument serialization.
  */
 class FilterDOMSerializeTestCase extends DrupalWebTestCase {

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.