Problem/Motivation

Changing the administrative name in views ui does not update the active page title and gives the following js error:
TypeError: response.siteName is undefined
core/modules/views_ui/js/ajax.js?v=8.0.0-dev
Line 35

Proposed resolution

Add siteName to the response.

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#28 2314443-28.patch1.98 KBLendude
#28 2314443-28-TEST_ONLY.patch1.44 KBLendude
#27 2314443-27.patch1.97 KBLendude
#27 2314443-27-TEST_ONLY.patch1.42 KBLendude
#26 reroll_diff_7-26.txt816 bytesimmaculatexavier
#26 2314443-26.patch554 bytesimmaculatexavier
#7 2314443-7.patch559 bytesolli
#6 2314443-6-b.patch1.56 KBolli
#1 2314443-1.patch1.8 KBolli
views_ui_replace_title.patch1.92 KBolli
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

olli’s picture

olli CreditAttribution: olli commented
FileSize
2314443-1.patch1.8 KB

Here's an alternative without a need for site name in the response.

olli’s picture

olli CreditAttribution: olli commented
Issue summary: View changes
olli’s picture

olli CreditAttribution: olli commented
Issue summary: View changes
Issue tags: +JavaScript

I wonder if #1 really makes a better effort to replace the <title> than the original code.

olli’s picture

olli CreditAttribution: olli commented
Issue summary: View changes
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Both variants seems to be equivalent problematic if some admin theme changes the way how page titles are generated. Well, in this case the user will just see a not-perfect title.
Nothing horrible.

Comparing the two versions I would prefer the first one, given that it does not include a complex reged in the javascript file, which is always a burden for maintenance.
What do you think, which one is the one you like?

olli’s picture

olli CreditAttribution: olli commented
FileSize
2314443-6-b.patch1.56 KB

To get rid of the site name and regex, could we use a simple string replace?

olli’s picture

olli CreditAttribution: olli commented
Issue summary: View changes
Issue tags: -JavaScript
FileSize
2314443-7.patch559 bytes

One more! Let's leave the js alone and just add the siteName to the response.

The difference between the first and the latest one is whether you want to do this:

+++ b/core/modules/views_ui/src/Form/Ajax/ViewsFormBase.php
@@ -166,7 +166,7 @@ public function getForm(ViewStorageInterface $view, $display_id, $js) {
-        $response->addCommand(new Ajax\ReplaceTitleCommand($form_state['#page_title']));
+        $response->addCommand(new Ajax\ReplaceTitleCommand($form_state['#page_title'], $this->config('system.site')->get('name')));

Pick either one.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs review » Reviewed & tested by the community

Even the other one has the better DI, semantically it seems better not having to specify the actual page title as well. So I would be fine with the current version of the patch.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs review 
+++ b/core/modules/views/src/Ajax/ReplaceTitleCommand.php
@@ -39,7 +39,8 @@ public function __construct($title) {
+      'siteName' => \Drupal::config('system.site')->get('name'),

Shouldn't we be escaping this? I don;t think we can just trust this value.

olli’s picture

olli CreditAttribution: olli commented

We need both the title and siteName without escaping to set the document.title. If we escape the siteName, the regex fails and if we escape the page title, it will get double escaped. This is also how it is in Views 7.x

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

Did you tried an xss::filter?

olli’s picture

olli CreditAttribution: olli commented
Issue tags: +JavaScript

Re #11: I guess that wouldn't work if my site name is <script>alert(0)</script>.

I don't see why we'd escape or filter the value we use to set document.title in js.

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom commented
Status: Needs review » Needs work

Patch here still applies, but I think is at needs work based on the above.

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

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should 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.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should 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.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should 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.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should 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.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should 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.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should 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.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.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: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should 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.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ as a volunteer commented
Version: 8.9.x-dev » 9.2.x-dev
Issue tags: -JavaScript

not a js issue, fix is on the php side

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.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Issue tags: +Bug Smash Initiative, +Needs tests, +Needs reroll
immaculatexavier’s picture

immaculatexavier CreditAttribution: immaculatexavier as a volunteer and at Srijan | A Material+ Company for Drupal India Association commented
Status: Needs work » Needs review
FileSize
2314443-26.patch554 bytes
reroll_diff_7-26.txt816 bytes

Rerolled patch against 9.4.x with diff

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at iO commented
Issue tags: -Needs tests, -Needs reroll
FileSize
2314443-27-TEST_ONLY.patch1.42 KB
2314443-27.patch1.97 KB

Here is a JS test for this, added a case with an unsafe title too to make sure this gets handled properly.

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at iO commented
FileSize
2314443-28-TEST_ONLY.patch1.44 KB
2314443-28.patch1.98 KB

Now with '===' and not '='

The test only patch is the interdiff.

The last submitted patch, 28: 2314443-28-TEST_ONLY.patch, failed testing. View results

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs review » Reviewed & tested by the community

Test and fix look good to me. Can't quite believe this has been broken for so long, almost wonder if this feature should be removed instead!

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 28: 2314443-28.patch, failed testing. View results

Lendude’s picture

Lendude
Primary language Dutch
Location Amsterdam
CreditAttribution: Lendude at iO commented
Status: Needs work » Reviewed & tested by the community

Unrelated fail

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 28: 2314443-28.patch, failed testing. View results

longwave’s picture

longwave
he/him
Primary language English
Location UK
CreditAttribution: longwave at Full Fat Things commented
Status: Needs work » Reviewed & tested by the community

Unrelated fail in QuickEditFileTest

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott at Thunder commented
Version: 9.4.x-dev » 9.3.x-dev
Status: Reviewed & tested by the community » Fixed

I tested this with XSS in both the site name and the views admin title - all works fine.

Committed and pushed fe3be7e13e to 10.0.x and b72a2b1194 to 9.5.x and abdb8f0024 to 9.4.x and d3c42ff26f to 9.3.x. Thanks!

  • alexpott committed fe3be7e on 10.0.x 
    Issue #2314443 by olli, Lendude, immaculatexavier, dawehner: Changing...

  • alexpott committed b72a2b1 on 9.5.x 
    Issue #2314443 by olli, Lendude, immaculatexavier, dawehner: Changing...

  • alexpott committed abdb8f0 on 9.4.x 
    Issue #2314443 by olli, Lendude, immaculatexavier, dawehner: Changing...

  • alexpott committed d3c42ff on 9.3.x 
    Issue #2314443 by olli, Lendude, immaculatexavier, dawehner: Changing...

Status: Fixed » Closed (fixed)

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