Not sure if this is releveant or not due to this issue: https://www.drupal.org/node/2278383

But if you have something like this:

  drupal_set_message('test 1', 'error');
  drupal_set_message('test 2', 'status');

Then the status message will be styled as an error message, but not as a status message.

CommentFileSizeAuthor
#18 interdiff.txt448 bytesstar-szr
#18 2348783-18.patch1.76 KBstar-szr
#16 fix-wrong-classes-on-status-messages-2348783-8.patch1.76 KBkugta
#16 interdiff-2348783-8-16.txt1.38 KBkugta
#8 fix-wrong-classes-on-status-messages-2348783.pass_.patch1.42 KBlarowlan
#8 2348783-status-issues.fail_.patch1.18 KBlarowlan
#5 fix-wrong-classes-on-status-messages-2348783.pass_.patch1.42 KBlarowlan
#5 2348783-status-issues.fail_.patch1.06 KBlarowlan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

leewillis77’s picture

leewillis77 CreditAttribution: leewillis77 commented
Status: Active » Needs review
FileSize
fix-wrong-classes-on-status-messages-2348783.diff5.73 KB

It appears that this is a twig template issue, as duplicate classes are added (Each message inherits the previous message's classes). So - the first message gets messages messages--error and the next gets messages messages--error messages--status

It seems that this is because the classes are added to a set of classes that is never reset in between messages.

The attached patch resolves this for me, but I'm not a twig expert, so there may be a better way to store/reset the attributes for each iteration through the loop.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented

I suspect a1a0cc86c086e8242701ef4a43fbf2398a139886

leewillis77’s picture

leewillis77 CreditAttribution: leewillis77 commented
FileSize
fix-wrong-classes-on-status-messages-2348783-2.diff395 bytes

Sorry - that previous patch included a bunch of irrelevant changes. Revised patch attached.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Issue tags: +Needs tests

tests coming

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
FileSize
2348783-status-issues.fail_.patch1.06 KB
fix-wrong-classes-on-status-messages-2348783.pass_.patch1.42 KB

should be red/green

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Issue tags: -Needs tests

The last submitted patch, 5: 2348783-status-issues.fail_.patch, failed testing.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
FileSize
2348783-status-issues.fail_.patch1.18 KB
fix-wrong-classes-on-status-messages-2348783.pass_.patch1.42 KB

meh merge head first lee

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented

The last submitted patch, 8: 2348783-status-issues.fail_.patch, failed testing.

kugta’s picture

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

Tested by setting six messages of three different types in random order. All the class names are rendered correctly.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented

This makes sense, maybe a comment in the template to explain though?

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Status: Reviewed & tested by the community » Needs review
pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented
Status: Needs review » Needs work 
  1. +++ b/core/modules/system/src/Tests/Theme/MessageTest.php
@@ -0,0 +1,39 @@
+/**
+ * @file
+ * Definition of Drupal\system\Tests\Theme\MessageTest.
+ */

    Replace this with "Contains Drupal\system\...". 

  2. +++ b/core/modules/system/src/Tests/Theme/MessageTest.php
@@ -0,0 +1,39 @@
+  /**
+   * Modules to enable.
+   *
+   * @var array
+   */
+  public static $modules = array('system');

    Use "{@inheritdoc}", this is already documented on the parent class.

kugta’s picture

kugta CreditAttribution: kugta commented
Assigned: Unassigned » kugta
kugta’s picture

kugta CreditAttribution: kugta commented
Assigned: kugta » Unassigned
Status: Needs work » Needs review
FileSize
interdiff-2348783-8-16.txt1.38 KB
fix-wrong-classes-on-status-messages-2348783-8.patch1.76 KB
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Status: Needs review » Reviewed & tested by the community

Comment only changes, back to rtbc

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
FileSize
2348783-18.patch1.76 KB
interdiff.txt448 bytes 
+++ b/core/modules/system/src/Tests/Theme/MessageTest.php
@@ -2,7 +2,7 @@
- * Definition of Drupal\system\Tests\Theme\MessageTest.
+ * Contains Drupal\system\Tests\Theme\MessageTest.

Contains \Drupal

Fixed here and leaving at RTBC because it's so minor.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 18: 2348783-18.patch, failed testing.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented

Thanks, reviewing patches on your phone--

skein queued 18: 2348783-18.patch for re-testing.

skein’s picture

skein CreditAttribution: skein commented
Status: Needs work » Reviewed & tested by the community

Not sure if I have the authority to do it, but tested the patch and all seems fine and the tests have passed. So I will put it in RTBC.

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented

@skein, yep thanks! Testbot is having a rough day :(

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Thanks for the fix and the test!

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 0803312 on 8.0.x 
    Issue #2348783 by larowlan, Cottser, kugta | skein: Fixed...

Status: Fixed » Closed (fixed)

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