Current theme function

Located in includes/theme.inc:

function theme_mark($variables) {
  $type = $variables['type'];
  global $user;
  if ($user->uid) {
    if ($type == MARK_NEW) {
      return ' ' . t('new') . '';
    }
    elseif ($type == MARK_UPDATED) {
      return ' ' . t('updated') . '';
    }
  }
}

Potential Changes

  • The tag should be changed to <mark>.
  • The class should be removed. It's superflous. The element speaks for itself and context can be derived from the wrapper.
  • The core/theme CSS needs to adjusted where targeting .marker. <mark> element has a yellow background and black foreground by default so we can remove the core styling from system.theme.css and style it however in the core themes, if at all.
  • The code itself could be cleaned up and simplified. Maybe it could take the string as an argument instead of dealing with constants. I say this because there are other places where it could be used easier if that were the case, such as comment.tpl.php:
    <?php if ($new): ?>
        <span class="new"><?php print $new ?></span>
      <?php endif; ?>
    

When to Use the MARK Element

  • Showing search results
  • Highlight text in a quotation
  • Indicate new items in a list
  • Show the current date in a calendar
  • Highlighting an error

References

http://webdesign.about.com/od/html5tags/a/understanding-the-mark-element...
https://www.w3.org/wiki/HTML/Elements/mark
https://developer.mozilla.org/en/docs/Web/HTML/Element/mark

Files: 

Comments

karschsp’s picture

Assigned: Unassigned » karschsp
Status: Active » Needs review
FileSize
3.32 KB
FAILED: [[SimpleTest]]: [MySQL] 33,650 pass(es), 8 fail(s), and 1,305 exception(es). View

Here's a first stab at this.

Status: Needs review » Needs work

The last submitted patch, 1311372-theme_mark.001.patch, failed testing.

karschsp’s picture

FileSize
3.93 KB
FAILED: [[SimpleTest]]: [MySQL] 33,806 pass(es), 0 fail(s), and 170 exception(es). View

Here's an updated patch for the /core change and removing one last MARK_NEW from common.inc.

karschsp’s picture

Status: Needs work » Needs review

needs review

Status: Needs review » Needs work

The last submitted patch, 1311372-theme_mark.003.patch, failed testing.

karschsp’s picture

Status: Needs work » Needs review
FileSize
4.49 KB
PASSED: [[SimpleTest]]: [MySQL] 33,815 pass(es). View

Missed a stray MARK_READ in that last patch.

Status: Needs review » Needs work

The last submitted patch, 1311372-theme_mark.006.patch, failed testing.

karschsp’s picture

Status: Needs work » Needs review

Re-queuing due to testbot failures seemingly unrelated to this patch.

karschsp’s picture

#6: 1311372-theme_mark.006.patch queued for re-testing.

Jacine’s picture

Status: Needs review » Needs work

Hey @karschsp, thanks so much for the patch! Sorry for taking so long to get to it.

+++ b/core/includes/theme.incundefined
@@ -1921,19 +1895,13 @@ function theme_tablesort_indicator($variables) {
+    return ' <mark>' . t($type) . '</mark>';

Hey, can we add the $type as a class here.

+++ b/core/includes/theme.incundefined
@@ -1921,19 +1895,13 @@ function theme_tablesort_indicator($variables) {
+    return ' <mark>' . t($type) . '</mark>';

Hey, can we add the $type as a class here.

cosmicdreams’s picture

on my list for tonight

cosmicdreams’s picture

Status: Needs work » Needs review
FileSize
4.16 KB
PASSED: [[SimpleTest]]: [MySQL] 34,793 pass(es). View

Rerolled and made the one-liner that jacine requested in #10

Niklas Fiekas’s picture

Status: Needs review » Needs work

Thank you for the patch.

+++ b/core/includes/theme.incundefined
@@ -2053,19 +2053,13 @@ function theme_tablesort_indicator($variables) {
+    return ' <mark class="' . t($type) . '">' . t($type) . '</mark>';

We should not use t() for class names. We also can't call t() on a variable -- the first parameter to t() must be a string literal. That's why there were MARK_NEW, MARK_UPDATED and MARK_READ and why they probably should stay.

Jacine’s picture

That's why there were MARK_NEW, MARK_UPDATED and MARK_READ and why they probably should stay.

Well maybe, but they are not good enough. This needs to be more flexible, or it's useless. This theme function isn't used everywhere it could be right now. It should be used for other things like a published/unpublished status as well. It's not flexible enough IMO. There has to be a way around this.

JohnAlbin’s picture

Assigned: karschsp » Unassigned
Status: Needs work » Needs review
FileSize
11.17 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/includes/theme.inc. View

I agree that MARK_NEW, etc should stay, but for completely different reasons. Drupal uses node_mark() to mark nodes and comments as read, new or updated. It's simplest if node_mark returns a constant rather then a translated string and a class name.

So let's make theme_mark more useful by having an optional $content variable which takes a translated string containing the text that should be placed inside the mark element. If $content is not given but a $type is given, then we put some default text into $content and wrap that in the mark element.

This new $content variable will allow theme_mark() to be used for any legitimate usage of the HTML5 mark element, e.g. marking a highlighted passage of an existing document.

I've removed the leading space from theme_mark because that was dumb and only needed in one place in Drupal core. I've also removed the global $user bit because it was redundant with the check done in node_mark() and because it was incredibly brain-dead to put business logic in a theme function.

I've also removed the implicit check for MARK_READ in theme_mark() before returning a themed mark. theme_mark() now assumes that if you called theme('mark') that you really did want a mark element. The check for MARK_READ (i.e. I do not want a mark element) is now done before theme('mark') is called.

This theme function isn't used everywhere it could be right now.

And now it is, I think. :-) I've made the comment.tpl's $new variable and the outdated translation marker use theme_mark.

So you can now use theme_mark in 2 ways:

  1. theme('mark', array('type' => MARK_NEW));
    
  2. theme('mark', array('content' => t('outdated'), 'attributes' => array('class' => array('outdated'))))
    

Status: Needs review » Needs work

The last submitted patch, 1311372-15-mark-html5.patch, failed testing.

JohnAlbin’s picture

FileSize
11.17 KB
PASSED: [[SimpleTest]]: [MySQL] 39,984 pass(es). View

Updated patch

JohnAlbin’s picture

Status: Needs work » Needs review

CNR

cosmicdreams’s picture

Status: Needs review » Reviewed & tested by the community

Looks good

webchick’s picture

Status: Reviewed & tested by the community » Needs work

This makes a lot of sense.

+++ b/core/includes/theme.incundefined
@@ -1982,24 +1982,31 @@ function theme_tablesort_indicator($variables) {
+ *   - content (optional): A string containing the text to mark.
...
+  $content = $variables['content'];

These two things seem to contradict each other. If $content is optional, then shouldn't we do an isset() check in that line below?

+++ b/core/includes/theme.incundefined
@@ -1982,24 +1982,31 @@ function theme_tablesort_indicator($variables) {
+ *   - attributes: The attributes applied to the mark element.

Is attributes required? Or is it also optional? I think it's optional, in which case it's weird that's not specified, where it is on type and content.

JohnAlbin’s picture

Status: Needs work » Needs review
FileSize
12.33 KB
PASSED: [[SimpleTest]]: [MySQL] 40,251 pass(es). View

These two things seem to contradict each other. If $content is optional, then shouldn't we do an isset() check in that line below?

Good question! But all 3 of those variables have default values set in hook_theme(). Note this hunk in the patch:

@@ -6598,7 +6598,7 @@ function drupal_common_theme() {
       'variables' => array('style' => NULL),
     ),
     'mark' => array(
-      'variables' => array('type' => MARK_NEW),
+      'variables' => array('type' => MARK_READ, 'content' => NULL, 'attributes' => array()),

So they will definitely exist when theme_mark is called.

Is attributes required? Or is it also optional? I think it's optional, in which case it's weird that's not specified, where it is on type and content.

Which means I'm not explaining the variables properly. Technically, most variables in the theme system will have default values and, thus, are "optional". I just rewrote that docblock to make it obvious how the "type" variable effects the other variables.

/**
 * Returns HTML for marked content; often used for new or updated content.
 *
 * @param $variables
 *   An associative array containing:
 *   - content: A string containing the text to mark.
 *   - attributes: The attributes applied to the mark element.
 *   - type: An optional integer representing the marker type to display; see
 *     MARK_NEW and MARK_UPDATED. If type is specified, the "content" does not
 *     need to be set since a default value will be provided for it.
 */
JohnAlbin’s picture

FileSize
11.27 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1311372-22-mark-html5.patch. Unable to apply patch. See the log in the details link for more information. View

Guh. Wrong patch. This one please.

sannejn’s picture

After applying the latest patch I get the following whitescreen-error:
Fatal error: Call to undefined function drupal_attributes() in /core/includes/theme.inc on line 2002

Is this a new function yet to be implemented?

JohnAlbin’s picture

Issue tags: -html5

#22: 1311372-22-mark-html5.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +html5

The last submitted patch, 1311372-22-mark-html5.patch, failed testing.

JohnAlbin’s picture

FileSize
9.89 KB
FAILED: [[SimpleTest]]: [MySQL] 48,738 pass(es), 1 fail(s), and 0 exception(s). View

Here's a re-rolled patch.

JohnAlbin’s picture

Status: Needs work » Needs review

Review!

Status: Needs review » Needs work
Issue tags: -html5

The last submitted patch, 1311372-26-mark-html5.patch, failed testing.

JohnAlbin’s picture

Status: Needs work » Needs review

#26: 1311372-26-mark-html5.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +html5

The last submitted patch, 1311372-26-mark-html5.patch, failed testing.

JohnAlbin’s picture

Title: theme_mark() should use the <mark> element, not <span> » Replace theme_mark() with <mark> element

After talking with Jen Lapton, we decided we should remove this entire theme hook.

JohnAlbin’s picture

Status: Needs work » Needs review
FileSize
0 bytes
FAILED: [[SimpleTest]]: [MySQL] 58,147 pass(es), 1 fail(s), and 0 exception(s). View

Here's the patch that removes the theme function.

JohnAlbin’s picture

FileSize
0 bytes
PASSED: [[SimpleTest]]: [MySQL] 57,845 pass(es). View

lol. totally wrong patch file.

JohnAlbin’s picture

FileSize
8.58 KB
FAILED: [[SimpleTest]]: [MySQL] 57,889 pass(es), 3 fail(s), and 0 exception(s). View

*sigh* Third time's a charm.

Status: Needs review » Needs work

The last submitted patch, 1311372-34-remove-theme-mark.patch, failed testing.

JohnAlbin’s picture

Status: Needs work » Needs review
FileSize
7.82 KB
FAILED: [[SimpleTest]]: [MySQL] 58,466 pass(es), 3 fail(s), and 0 exception(s). View

Re-roll.

Status: Needs review » Needs work

The last submitted patch, 1311372-36-remove-theme-mark.patch, failed testing.

andypost’s picture

@JohnAlbin I've trying to refactor theme_mark() in another issue #2062097: Remove calls to deprecated global $user in theme.inc and came to nearly the same idea.

The function needed for node, comment, tracker and translation modules so should be shared because comment module could be used for any entity soon #1907960-381: Helper issue for "Comment field"

The only thing I suggest is remove access check from theme function

joelpittet’s picture

Issue summary: View changes

Updated issue summary.

joelpittet’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
6.38 KB
FAILED: [[SimpleTest]]: [MySQL] 59,114 pass(es), 3 fail(s), and 0 exception(s). View

Re-rolled again.

Status: Needs review » Needs work

The last submitted patch, 39: 1311372-39-theme-mark-tag.patch, failed testing.

mdrummond’s picture

Status: Needs work » Needs review
FileSize
4.16 KB
FAILED: [[SimpleTest]]: [MySQL] 63,148 pass(es), 3 fail(s), and 0 exception(s). View

Re-rolled the patch to apply cleanly against 8.x.

Status: Needs review » Needs work

The last submitted patch, 41: replace-theme-mark-2021161-48.patch, failed testing.

Wim Leers’s picture

You'll want to update HistoryTimestampTest, i.e. replace

$result = $this->xpath('//span[@class=:class]', array(':class' => 'marker'));

with

$result = $this->xpath('//span[@class=:class]', array(':class' => 'mark--new'));


Questions:

  • why not have a render element type, like the issue title says?
  • why not apply a general class in all cases?
mgifford’s picture

Issue tags: +Needs reroll
mgifford’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
4.35 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 70,471 pass(es), 3 fail(s), and 0 exception(s). View
markcarver’s picture

Status: Needs review » Needs work
Related issues: +#1939092: Convert theme_mark() to Twig
+++ b/core/modules/tracker/tracker.pages.inc
@@ -80,19 +80,22 @@ function tracker_page($account = NULL) {
-      $mark_build = array(
-        '#theme' => 'mark',
-        '#status' => node_mark($node->id(), $node->getChangedTime()),
-      );
...
-        'title' => array('data' => l($node->getTitle(), 'node/' . $node->id()) . ' ' . drupal_render($mark_build)),
+        'title' => array('data' => l($node->getTitle(), 'node/' . $node->id())),
...
+        $row['title']['data'] .= ' <mark class="mark--new">' . t('new') . '</mark>';
...
+        $row['title']['data'] .= ' <mark class="mark--updated">' . t('updated') . '</mark>';

We're losing the render array structure by adding in static markup. This makes it difficult to replace (or change classes accordingly).

In retrospect, these table rows really should be using a render array for the data/link instead of l(). That would allow us to append the render array with a new "marker" renderable child. This would allow us to replace just the $row['title']['data']['marker']['#markup'] instead of having to do a preg_replace() on the string.

Also, FWIW, we're kind of duplicating the efforts in #1939092: Convert theme_mark() to Twig... although this issue is completely removing the "mark" template entirely (which isn't a really good idea IMO). Twig caches everything so having a separate "mark" theme hook for the <mark> tag makes sense. It shouldn't be subject to the MARKED_NEW and MARKED_UPDATE constants though, it should be abstracted so the classes are added via preprocess and not in the template:

<mark{{attributes}}>{{value}}</mark>

The last submitted patch, 45: replace-theme-mark-2021161-45.patch, failed testing.

mgifford’s picture

Happy to mark this as a duplicate and focus efforts on #1939092: Convert theme_mark() to Twig. I was just trying to nudge this issue along.

markcarver’s picture

Title: Replace theme_mark() with <mark> element » Use <mark> element for 'mark' theme hook
Status: Needs work » Postponed

I think this should be a separate issue (as it's changing the markup), so just postponing it until that issue is resolved.

markcarver’s picture

Issue tags: +Twig, +dreammarkup

Forgot to some other issue cleanup

mgifford’s picture

Status: Postponed » Needs work
andypost’s picture

It's not clear what is a current proposal after #1939092: Convert theme_mark() to Twig

akalata’s picture

@andypost I think the distinction is clear; this issue is about the markup, and that issue was about how to deliver the markup.

mgifford’s picture

Status: Needs work » Closed (duplicate)

I'm going to mark this as a duplicate as it's being addressed over here #867830: "unpublished" status of rendered entities is not accessible (and looks bad). Can we focus our efforts on this patch?

mgifford’s picture

Status: Closed (duplicate) » Needs work

Ok, we're moving this back here.

joelpittet’s picture

Version: 8.0.x-dev » 8.1.x-dev
Status: Needs work » Needs review
FileSize
5.05 KB

Crap we missed this, the only tricky part is maybe getting this logged_in status check moved out of the template and onto the render array.

Here's a rough conversion that will likely fail, and I'm not supposed to change stable/classy so likely have to revert that change later... just want to kick the tires on this a bit...

And there is a bug fix in here now too AND this likely doesn't work well with render cache as it probably needs some cache tags and context when it's being built.

Status: Needs review » Needs work

The last submitted patch, 56: use_mark_element_for-1311372-56.patch, failed testing.

lauriii’s picture

+++ b/core/modules/system/templates/mark.html.twig
@@ -9,14 +9,9 @@
+<mark{{ attributes }}>{{ text }}</mark>

Do we want to show the mark element even when the text is empty?

dawehner’s picture

+++ b/core/includes/theme.inc
@@ -468,6 +468,38 @@ function theme_settings_convert_to_config(array $theme_settings, Config $config)
+      $variables['attributes']['class'][] = 'mark--new';
...
+      $variables['attributes']['class'][] = 'mark--updated';
...
+      $variables['attributes']['class'][] = 'mark--outdated';

I'm curious whether this logic should/could be part of the template instead?

mikemiles86’s picture

Assigned: Unassigned » mikemiles86
mikemiles86’s picture

To address comment #58 by @lauriii , based on all my digging into standards around the tag it is valid for it to be empty. don't think we'd want to hide it if text is empty.

@dawehner keeping that logic within theme.inc seems like a cleaner approach then moving it into the template. If moved into the template, we would end up with a template looking something like this:

{% if status is constant('MARKUP_NEW') %}
    <mark{{ attributes.addClass('markup--new') }}>{{ text }}</mark>
{% elseif status is constant('MARKUP_UPDATED') %}
    <mark{{ attributes.addClass('markup--updated') }}>{{ text }}</mark>
{% elseif status is constant('MARKUP_READ') %}
  <mark{{ attributes.addClass('markup--outdated') }}>{{ text }}</mark>
{% else %}
    <mark{{ attributes }}>{{ text }}</mark>
{% endif %}

That seems like a not-ideal scenario.

I have made an update to patch #56. Noticed that the render method in HistoryUserTimestamp.php was using the depreciated drupal_render().

mikemiles86’s picture

Add this time to attach the correct patch file...

mikemiles86’s picture

Assigned: mikemiles86 » Unassigned
Status: Needs work » Needs review
Issue tags: +SprintWeekend2016, +SprintWeekendBOS

Status: Needs review » Needs work

The last submitted patch, 62: use_mark_element_for-1311372-62.patch, failed testing.

dawehner’s picture

  1. +++ b/core/includes/theme.inc
    @@ -468,6 +468,38 @@ function theme_settings_convert_to_config(array $theme_settings, Config $config)
    + */
    +function template_preprocess_mark(&$variables) {
    +  switch ($variables['status']) {
    +    case MARK_NEW:
    +      $variables['text'] = t('New');
    +      $variables['attributes']['class'][] = 'mark--new';
    +      break;
    +    case MARK_UPDATED:
    +      $variables['text'] = t('Updated');
    +      $variables['attributes']['class'][] = 'mark--updated';
    +      break;
    +    case MARK_READ:
    +      $variables['text'] = t('Read');
    +      $variables['attributes']['class'][] = 'mark--outdated';
    

    This looks something which could be totally moved to the template. Well these days we try to basically remove preprocess functions as much as possible.

  2. +++ b/core/modules/node/src/NodeListBuilder.php
    @@ -105,7 +105,8 @@ public function buildRow(EntityInterface $entity) {
    -      '#mark_type' => node_mark($entity->id(), $entity->getChangedTime()),
    +      '#status' => node_mark($entity->id(), $entity->getChangedTime()),
    +      '#access' => (\Drupal::currentUser()->isAuthenticated() && ($mark == MARK_NEW || $mark == MARK_UPDATED)),
         );
    

    This would need a cache context so it changes between authenticated and non authenticated users.

lauriii’s picture

  • Moved the logic back to template.
  • Reverted changes made for Classy and Stable because of BC.
  • Added cache context for the user permissions.
joelpittet’s picture

+++ b/core/modules/system/templates/mark.html.twig
@@ -13,10 +13,18 @@
 {% if logged_in %}

Let's please move the logged_in check out of this template, that logic doesn't need to be representative of a mark tag, it should be more controller logic deciding when to use that element.

lauriii’s picture

Rerolled new patch without macros and added missing comma for HistoryUserTimestamp

The last submitted patch, 66: use_mark_element_for-1311372-66.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 68: use_mark_element_for-1311372-67.patch, failed testing.

joelpittet’s picture

Status: Needs work » Needs review
FileSize
3.59 KB
2.3 KB

Thank you @lauriii. Here's an iteration on that and fix the missing variable bug and removes the markup changes in stable.

Manuel Garcia’s picture

Status: Needs review » Reviewed & tested by the community

This looks beautiful to me, issues mentioned on #65 are addressed as well.

Wim Leers’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs screenshots
  1. +++ b/core/modules/history/src/Plugin/views/field/HistoryUserTimestamp.php
    @@ -100,8 +100,15 @@ public function render(ResultRow $values) {
    +        '#cache' => array(
    +          'contexts' => array(
    +            'user.permissions',
    +          ),
    +        ),
    
    +++ b/core/modules/node/src/NodeListBuilder.php
    @@ -103,9 +103,11 @@ public function buildHeader() {
    +      '#access' => (\Drupal::currentUser()->isAuthenticated() && ($node_mark == MARK_NEW || $node_mark == MARK_UPDATED)),
    

    This cache context does not match the logic. It describes far more variations than what's actually going on here: a binary "anon or auth" variation.

    The proper cache context to use here is user.roles:authenticated.

  2. +++ b/core/modules/system/templates/mark.html.twig
    @@ -13,10 +13,10 @@
    -{% if logged_in %}
    -  {% if status is constant('MARK_NEW') %}
    -    {{ 'New'|t }}
    -  {% elseif status is constant('MARK_UPDATED') %}
    -    {{ 'Updated'|t }}
    -  {% endif %}
    +{% if status is constant('MARK_NEW') %}
    +  <mark{{ attributes }}>{{ 'New'|t }}</mark>
    +{% elseif status is constant('MARK_UPDATED') %}
    +  <mark{{ attributes }}>{{ 'Updated'|t }}</mark>
    +{% elseif status is constant('MARK_READ') %}
    +  <mark{{ attributes }}>{{ 'Read'|t }}</mark>
    

    SO MUCH BETTER!

  3. +++ b/core/themes/stable/templates/content/mark.html.twig
    @@ -16,5 +16,7 @@
    +  {% elseif status is constant('MARK_READ') %}
    +    {{ 'Read'|t }}
    

    Except this is new? We never had Read markers before. This is a huge visual change. Let's have screenshots to properly assess this before committing.

Wim Leers’s picture

Note the cache context was okay: it would have ensured correct render caching. But it is not optimal, because it causes more variations than necessary.

So, this is not about security or correctness, this is about efficiency.

mikemiles86’s picture

+++ b/core/modules/system/templates/mark.html.twig
@@ -13,10 +13,10 @@
-{% if logged_in %}
-  {% if status is constant('MARK_NEW') %}
-    {{ 'New'|t }}
-  {% elseif status is constant('MARK_UPDATED') %}
-    {{ 'Updated'|t }}
-  {% endif %}
+{% if status is constant('MARK_NEW') %}
+  <mark{{ attributes }}>{{ 'New'|t }}</mark>
+{% elseif status is constant('MARK_UPDATED') %}
+  <mark{{ attributes }}>{{ 'Updated'|t }}</mark>
+{% elseif status is constant('MARK_READ') %}
+  <mark{{ attributes }}>{{ 'Read'|t }}</mark>

Are we getting rid of the 'mark-XXX' classes? Or should this section be updated to be
{{ attributes.addClass('mark-XXX') }}

lauriii’s picture

Status: Needs work » Needs review
Issue tags: -Needs screenshots
FileSize
3.6 KB
558 bytes

#73.3: It is not actually a change for core's use cases because we never actually show that when it is read.
'#access' => ($mark == MARK_NEW || $mark == MARK_UPDATED)

#75: We didn't use to have any classes in core so this doesn't affect the situation at all. What we do is simply wrap the text with mark element.

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community

Interesting/fair.

catch’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/node/src/NodeListBuilder.php
@@ -103,9 +103,11 @@ public function buildHeader() {
+      '#access' => (\Drupal::currentUser()->isAuthenticated() && ($node_mark == MARK_NEW || $node_mark == MARK_UPDATED)),

Doesn't this also need the user.roles.authenticated cache context?

catch’s picture

Also while I'm here, the docs for mark don't make me 100% confident it's the right thing here for 'new'/'updated'. https://developer.mozilla.org/en/docs/Web/HTML/Element/mark

i.e. we should definitely be using it in https://api.drupal.org/api/drupal/core%21modules%21search%21search.modul... instead of strong tags because that's explicitly about highlighting a particular section of text based on the context that it's being shown in a search result for the word that's highlighted.

I don't see the same applicability to new/updated.

Of course if we don't use it, that makes the 'mark' element very unfortunately named.

Wim Leers’s picture

#78: that is already there, see #76.

catch’s picture

@WimLeers I see it in HistoryUserTimestamp but not in NodeListBuilder?

Cottser’s picture

Since it hasn't really been discussed on this issue I'd love to see a response to #79. I did a brief search and turned up this:

Indicate new items in a list—If you have a list of items that is regularly being updated, you can indicate the new items with the MARK element to make it easier for readers to see what’s been added.

http://webdesign.about.com/od/html5tags/a/understanding-the-mark-element...

Not sure I'd consider webdesign.about.com the best resource but at least it's not totally unheard of…

mgifford’s picture

Issue summary: View changes

From the W3C: "The element represents a run of text in one document marked or highlighted for reference purposes, due to its relevance in another context. "

Hmm.. Yes.. This doesn't quite seem what what they meant. New & Updated are useful to highlight, but don't seem to fit well with examples. Even "Indicate new items in a list" would probably be a matter of wrapping the string with the new item, rather than attaching a New behind it.

Updating the summary with some references.

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.

mdrummond’s picture

Here's another article about the mark element: HTML5 Doctors: Draw attention with mark.

One of the items in the W3C definition:

In the main text, the highlighted text typically marks text that may be of special relevance for the user's current activity, like search results.

Coming to a page and seeing items that are new or updated seems like items of special relevance to a user's current activity. There isn't always a perfect HTML element to convey the right semantics, and I can see that the mark element may be a bit of a stretch here. That said, I think the mark element conveys better and more useful semantics than a simple span, so I think it is worth making the change.

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.