Problem/Motivation

On a high level perspective there are examples like blocks which should return immutable HTML fragments, but other places, like the HTML page which should be mutable. Currently the HtmlFragment is mutable (totally fine) and the HtmlFragmentInterface is not (also fine from a high level perspective, you can argue here but it is the case atm.).

Proposed resolution

Introduce a MutableHtmlFragmentInterface which will also be implemented by the HtmlFragment which contains methods like setTitle().

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#11 2339475-Mutable-HTML-fragment-interface.patch10.93 KBfgm
#8 2339475-Mutable-HTML-fragment-interface.patch11.21 KBfgm
#2 2339475-Introduce-a-mutable-HTML-fragment-interface.patch6.84 KBfgm
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Crell’s picture

Crell CreditAttribution: Crell commented
Parent issue: » #2327277: [Meta] Page rendering meta discussion
fgm’s picture

fgm
Primary language French
Location Paris, France
CreditAttribution: fgm commented
Status: Active » Needs review
FileSize
2339475-Introduce-a-mutable-HTML-fragment-interface.patch6.84 KB

Here is one possible implementation. I'm not totally sold on this, though :

  • the setter methods return $this, for a fluent interface, but :
    • this is not actually used by callers of these methods
    • within the mutable interface, $this makes little sense : the returned implementation is used for both its mutable and immutable interfaces, so which one should be returned ?
  • there is only one function (yet ?) in which it has been possible to use this interface instead of the concrete class : in all other cases, the methods are invoked on a locally create concrete Page or Fragment

But at least it seems to work.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented 
+++ b/core/lib/Drupal/Core/Page/HtmlMutableFragmentInterface.php
@@ -0,0 +1,80 @@
+interface HtmlMutableFragmentInterface {

Should this extend the normal one? Just wondering.

Status: Needs review » Needs work

The last submitted patch, 2: 2339475-Introduce-a-mutable-HTML-fragment-interface.patch, failed testing.

fgm’s picture

fgm
Primary language French
Location Paris, France
CreditAttribution: fgm commented

@dawehner : yes, that would obviously be a tempting choice, but then it would violate the basic principle of interface segregation.

I do not see how this change can break the TermTranslationFieldViewTest, though. Especially as it passes on my machine. Bot fluke ?

Status: Needs work » Needs review

fgm queued 2: 2339475-Introduce-a-mutable-HTML-fragment-interface.patch for re-testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

@crell agreed that we should extend the interface.

TermTranslationFieldViewTest

test was broken.

fgm’s picture

fgm
Primary language French
Location Paris, France
CreditAttribution: fgm commented
FileSize
2339475-Mutable-HTML-fragment-interface.patch11.21 KB

Rerolled accordingly. This turned out to allow more extensive use of the interface. Let's see if it breaks any tests.

Crell’s picture

Crell CreditAttribution: Crell commented 
  1. +++ b/core/lib/Drupal/Core/Page/HtmlMutableFragmentInterface.php
@@ -0,0 +1,80 @@
+ * Mutator interface for the domain object for a portion of an HTML page, including related data.

    80 chars. :-( We can probably just drop everything after the comma. 

  2. +++ b/core/lib/Drupal/Core/Page/HtmlMutableFragmentInterface.php
@@ -0,0 +1,80 @@
+ * Related data includes any additional information relevant to a fragment of
+ * HTML that would not be part of the HTML string itself. That includes, for
+ * example, required CSS files, Javascript files, link tags, meta tags, and the
+ * title of a page or page section.

    We don't need to repeat this from the parent interface. 

  3. +++ b/core/lib/Drupal/Core/Page/HtmlMutableFragmentInterface.php
@@ -0,0 +1,80 @@
+ * @ingroup menu

    Er, no?

Just doc fixes, easy to address. Then we're good here.

Crell’s picture

Crell CreditAttribution: Crell commented
Status: Needs review » Needs work
fgm’s picture

fgm
Primary language French
Location Paris, France
CreditAttribution: fgm commented
Status: Needs work » Needs review
FileSize
2339475-Mutable-HTML-fragment-interface.patch10.93 KB

Rerolled accordingly. Actually, there was an actual earlier bug : a reference to Title::CHECK_PLAIN without its namespace, so I added it too.

Crell’s picture

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

Unless bot disapproves. Thanks, fgm!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 11: 2339475-Mutable-HTML-fragment-interface.patch, failed testing.

Status: Needs work » Needs review

fgm queued 11: 2339475-Mutable-HTML-fragment-interface.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 11: 2339475-Mutable-HTML-fragment-interface.patch, failed testing.

fgm’s picture

fgm
Primary language French
Location Paris, France
CreditAttribution: fgm commented

The single test which fails on the bot passes on my machine, and does not seem related to the patch in any way. This looks like a bot fluke to me : can anyone triple check ? It was not possibly to check it on simplytest.me, because displaying the testing page eats more than the 256 MB allocated to test instances, even with the minimal profile.

Status: Needs work » Needs review

fgm queued 11: 2339475-Mutable-HTML-fragment-interface.patch for re-testing.

Crell’s picture

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

Looks like it passed again. Bot fluke.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Version: 8.0.x-dev » 8.1.x-dev
Status: Reviewed & tested by the community » Postponed

#2352155: Remove HtmlFragment/HtmlPage will remove HtmlFragment/HtmlPage for now. Some of the concepts could be re-introduced in 8.1.x or later if we can find use cases. Moving to 8.1.x/postponed for now.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented

Which is probably more likely to be baby steps towards #1843798: [meta] Refactor Render API to be OO.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.