On search result pages, Drupal lists all found content, independent of the node's language. So search results could contain results of multiple languages. According to WCAG 2.0, language attributes should be used to identify changes in human language.

So instead of:

<dt>Search title in english</dt>
<dd>Search snippet in english</dd>
<dt>Search title in dutch</dt>
<dd>Search snippet in dutch</dd>

we should use:

<dt>Search title in english</dt>
<dd>Search snippet in english</dd>
<dt lang="nl">Search title in dutch</dt>
<dd lang="nl">Search snippet in dutch</dd>

when the current site language is set to English. If set to Dutch, only the English results should contain the lang attribute.

CommentFileSizeAuthor
#35 drupal.search-result-language.35.patch5.33 KBsun
#16 867114-langnone.patch4.48 KBjhodgdon
#9 867114-9.patch4.44 KBmgifford
#6 867114.patch4.28 KBjhodgdon

Comments

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Version: 6.9 » 7.x-dev

That is a valid point. It needs to be done in Drupal 7 first, then ported back to Drupal 6 if possible.

I would add that in Drupal 8, I am hoping that searches will be language-dependent anyway.

I would also add that most multilingual sites should be using the Internationalization module, and that module I believe restricts search results to the current language, making this unnecessary.

BarisW’s picture

BarisW commented

Too bad, I'm using the Internationalization module as well.
But that doesn't seem to happen, it returns Dutch nodes on the English site as well.. :(

Edit: After disabling the Acquia SOLR module, content is being excluded to the current language only. Maybe a bug of the Acquia SOLR module. I'll investigate.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Solr is completely separate from the core Search module and also I think doesn't rely on Internationalization.

I think with Internationalization you may need to enable one of its sub-modules, but I cannot remember which, in order to exclude content from search results by language. But it should work if you get the right modules enabled and/or settings.

BarisW’s picture

BarisW commented

I got it working using this hook: http://windmill.sk/project/module/apachesolr_language_filter

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Assigned: Unassigned » jhodgdon

I did a bit more researching on this.

It appears that the default HTML produced by modules/system/html.tpl.php (which is not overridden by any core theme) is:

<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="<?php print $language->language; ?>" version="XHTML+RDFa 1.0" dir="<?php print $language->dir; ?>"<?php print $rdf_namespaces; ?>>

So it is using the xml:lang attribute rather than the lang attribute -- see above reference -- XHTML prefers the xml:lang tag to the lang attribute, so this is correct.

So any search result item with a different language attribute from the site-wide global $language->language should be tagged as such.

HOWEVER, I think only the title and body snippet parts of the search result should be tagged, not the entire search result, since the other parts would be in the site-wide language (e.g. the node type, posted by information, etc.).

I'll put together a patch.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Active » Needs review
Issue tags: +Accessibility, +WCAG
StatusFileSize
new 867114.patch4.28 KB

OK, here's a patch. It appears to work on my test box. One note: I decided to name the template variable "$result_language", because I didn't want to confuse it with the global $language.

I'm not sure about writing a test for this, any ideas on how to detect that the xml:lang tag was put into the right place?

This patch produces the following HTML for the search results, in the case that the first item is English (site default) and second item is Spanish (change of language). I put the word "Jennifer" into both nodes, and searched on "Jennifer", so "Jennifer" is highlighted. Bird is the name of my testing box, and "jtest" is the content type.

  <ol class="search-results node-results">
    <li>
  <h3 class="title">
    <a href="http://bird/~eclipsework/drupal7/node/1">Test 1</a>

  </h3>
  <div class="search-snippet-info">
          <p class="search-snippet">        testing <strong>Jennifer</strong>  
        ...</p>
              <p class="search-info">jtest - <a href="/~eclipsework/drupal7/user/1" title="View user profile." class="username">jhodgdon</a> - 08/10/2010 - 15:40</p>

      </div>
</li>
<li>
  <h3 class="title">
    <a href="http://bird/~eclipsework/drupal7/node/2" xml:lang="es">nueva cosa</a>
  </h3>
  <div class="search-snippet-info">
          <p class="search-snippet" xml:lang="es">        prueba <strong>Jennifer</strong>  
        ...</p>

              <p class="search-info">jtest - <a href="/~eclipsework/drupal7/user/1" title="View user profile." class="username">jhodgdon</a> - 08/10/2010 - 15:40</p>
      </div>
</li>
  </ol>
BarisW’s picture

BarisW commented

Looks very good. I'm on vacation right now, will test it when I'm back. Is this written for 7.x?
Thanks!

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Yes, this is a 7.x fix.

mgifford’s picture

mgifford
he/him
Primary language English
commented
StatusFileSize
new 867114-9.patch4.44 KB

Ok, I've re-rolled the patch to keep up with HEAD. I've also got it in my sandbox here:
http://drupal7.dev.openconcept.ca/search/node/meus

Thoughts?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Still looks fine to me, but of course I wrote the original patch - thanks for the reroll.
Can we have a second opinion so we can RTBC?

BarisW’s picture

BarisW commented
Status: Needs review » Needs work

I see two strange things happening:

<li>
  <h3 class="title"><a href="http://127.0.0.1/drupal7/node/5" xml:lang="und">A language neutral node test</a></h3>
  <div class="search-snippet-info">
         <p class="search-snippet" xml:lang="und">...  eBay?  
            Add new comment    
      Tags:         <strong>test</strong>        no-language          ...</p>
              <p class="search-info"><a href="/drupal7/user/1" title="View user profile." class="username" xml:lang="" about="/drupal7/user/1" typeof="sioc:UserAccount" property="foaf:name">BarisW</a> - 08/23/2010 - 17:48 - 0 comments</p>
      </div>
</li>
<li>
  <h3 class="title"><a href="http://127.0.0.1/drupal7/node/3" xml:lang="en">Another english node</a></h3>
  <div class="search-snippet-info">
          <p class="search-snippet" xml:lang="en">...  eBay?  
            Add new comment    
      Tags:         <strong>test</strong>        english          ...</p>
              <p class="search-info"><a href="/drupal7/user/1" title="View user profile." class="username" xml:lang="" about="/drupal7/user/1" typeof="sioc:UserAccount" property="foaf:name">BarisW</a> - 08/23/2010 - 17:47 - 0 comments</p>
      </div>
</li>

- The username has an empty language tag. Not if it is caused by this patch or if this is another Drupal core bug.
- For language neutral nodes, it outputs xml:lang="und". It would then better not output the language tag?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Thanks for testing!

- I am not sure where the user name xml:lang is being set up, but I don't think it is from this patch. The user name is being formatted in http://api.drupal.org/api/function/node_search_execute/7 using http://api.drupal.org/api/function/theme_username/7. Maybe this mess you are seeing is coming from your theme? I don't see anything in here either: http://api.drupal.org/api/function/template_preprocess_username/7 and I didn't see that kind of HTML when I ran it on my test box.

- On the "und" - right! The node module should only output the language tag if the node is not equal to http://api.drupal.org/api/constant/LANGUAGE_NONE/7. That needs to be fixed in the patch.

BarisW’s picture

BarisW commented

The empty 'lang' tag is added by the RDF module. If you enable it (which is part of the default installation profile) you'll see them appearing. Leave that for now, as it's another issue.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

OK. Please file a separate issue for the RDF problem.

So the one thing that needs to be fixed from the #9 patch is to omit the language [EDIT]tag flag from the search result array generated by the node module[/EDIT] if the node language is LANGUAGE_NONE.

BarisW’s picture

BarisW commented

Yeah, that should do it!

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new 867114-langnone.patch4.48 KB

Here's a go at a new patch. (eek: confession - I just edited the previous patch)

BarisW’s picture

BarisW commented

Just a minor thing; Mark Boulton explained today that a border around a box is unneeded if it already has a background. A background alone would do to group items. That said: it looks great!

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

borders? boxes? This issue has nothing to do with CSS. Please file a different issue if you want to change styles of something.

BarisW’s picture

BarisW commented

Apologies. Wrong thread!

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

Ah. :) Well, we still need a review of the patch in #16

BarisW’s picture

BarisW commented

Since my laptop has been stolen during DrupalCon it's hard for me to test your patch. Tomorrow I've a laptop to use by the D.A. If anyone else wants to test this, please do so!

BarisW’s picture

BarisW commented

I tested the patch in #16 and it's looking great. Exactly what's needed. Another accessibility win!
I've tested with English, Dutch and Language-neutral. This is the output:

<ol class="search-results node-results"> 
<li> 
  <h3 class="title"> 
    <a href="http://localhost/drupal7/?q=node/1">NL test</a> 
  </h3> 
  <div class="search-snippet-info"> 
    <p class="search-snippet"><strong>Lorem</strong> ipsum dolor sit amet, consectetur adipiscing elit. Aliquam eleifend ...  Donec bibendum fermentum sollicitudin. Pellentesque nulla <strong>lorem</strong>, rutrum ut commodo ut, tincidunt euismod libero. Etiam ac felis non odio ...  vel turpis. Suspendisse potenti. Aliquam ullamcorper dui non <strong>lorem</strong> blandit pretium. Pellentesque tincidunt fermentum quam condimentum ...</p> 
    <p class="search-info"><a href="/drupal7/?q=user/1" title="View user profile." class="username" xml:lang="" about="/drupal7/?q=user/1" typeof="sioc:UserAccount" property="foaf:name">BarisW</a> - 08/27/2010 - 14:39 - 0 comments</p> 
  </div> 
</li> 
<li> 
  <h3 class="title"> 
    <a href="http://localhost/drupal7/?q=node/2" xml:lang="en">English test</a> 
  </h3> 
  <div class="search-snippet-info"> 
    <p class="search-snippet" xml:lang="en"><strong>Lorem</strong> ipsum dolor sit amet, consectetur adipiscing elit. Aliquam eleifend ...  Donec bibendum fermentum sollicitudin. Pellentesque nulla <strong>lorem</strong>, rutrum ut commodo ut, tincidunt euismod libero. Etiam ac felis non odio ...  vel turpis. Suspendisse potenti. Aliquam ullamcorper dui non <strong>lorem</strong> blandit pretium. Pellentesque tincidunt fermentum quam condimentum ...</p> 
    <p class="search-info"><a href="/drupal7/?q=user/1" title="View user profile." class="username" xml:lang="" about="/drupal7/?q=user/1" typeof="sioc:UserAccount" property="foaf:name">BarisW</a> - 08/27/2010 - 14:40 - 0 comments</p> 
  </div> 
</li> 
<li> 
  <h3 class="title"> 
    <a href="http://localhost/drupal7/?q=node/3">Language neutral test</a> 
  </h3> 
  <div class="search-snippet-info"> 
    <p class="search-snippet"><strong>Lorem</strong> ipsum dolor sit amet, consectetur adipiscing elit. Aliquam eleifend ...  Donec bibendum fermentum sollicitudin. Pellentesque nulla <strong>lorem</strong>, rutrum ut commodo ut, tincidunt euismod libero. Etiam ac felis non odio ...  vel turpis. Suspendisse potenti. Aliquam ullamcorper dui non <strong>lorem</strong> blandit pretium. Pellentesque tincidunt fermentum quam condimentum ...</p> 
    <p class="search-info"><a href="/drupal7/?q=user/1" title="View user profile." class="username" xml:lang="" about="/drupal7/?q=user/1" typeof="sioc:UserAccount" property="foaf:name">BarisW</a> - 08/27/2010 - 14:40 - 0 comments</p> 
  </div> 
</li> 
</ol>

We need at least one more user to test it before we can set it RTBC.

To test this

First test current state
- Enable the Locale and Content Translation modules
- Add another language
- Edit a content type (eg article) and under Publishing options, select Multilingual with translation
- Create a few nodes with several languages (english, language neutral and the added language).
- Run cron
- Search on your site and see that the results don't add the xml-lang tag to the H3 of the search results.

Apply patch and notice the difference
- Apply the patch of #16
- Place another search and check the H3 tag.

mgifford’s picture

mgifford
he/him
Primary language English
commented

Ok, I'm almost ready to RTBC this patch. I've got it up and running here http://drupal7.dev.openconcept.ca/search/node/fake

The concern I have is with adding a lot of blank xml:lang="" attributes to our HTML if either the search returns the same language as the page language or there is no language set. Is there any reason in RDFa specs where a blank xml:lang spec should be sent? Any advantages or disadvantages of this? Would be pretty easy to slightly alter this to verify if $result_language is empty or not.

   <h3 class="title">
-    <a href="<?php print $url; ?>"><?php print $title; ?></a>
+    <a href="<?php print $url; ?>"<?php if ($result_language) : ?> xml:lang="<?php print $result_language; ?>"<?php endif; ?>><?php print $title; ?></a>
   </h3>
   <div class="search-snippet-info">
     <?php if ($snippet) : ?>
-      <p class="search-snippet"><?php print $snippet; ?></p>
+      <p class="search-snippet"<?php if ($result_language) : ?> xml:lang="<?php print $result_language; ?>"<?php endif; ?>><?php print $snippet; ?></p>
     <?php endif; ?>

It is looking nice, this is my only concern.

BarisW’s picture

BarisW commented

The issue with the blank xml:lang="" tags is another issue, this has nothing to do with this patch. Looks like Drupal adds theme to the username? See #11 and #12.

mgifford’s picture

mgifford
he/him
Primary language English
commented

Is it wrong? Generally I don't like empty tags? I'm fine with dealing with that in another instance, but would just like to get an authoritative view on that issue.

Other than this I think it's RTBC.

BarisW’s picture

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

Great, thanks for the review and for the patch!

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Reviewed & tested by the community » Needs work 
+++ modules/search/search-result.tpl.php	22 Aug 2010 19:54:39 -0000
@@ -31,29 +33,31 @@
-    <a href="<?php print $url; ?>"><?php print $title; ?></a>
+    <a href="<?php print $url; ?>"<?php if ($result_language) : ?> xml:lang="<?php print $result_language; ?>"<?php endif; ?>><?php print $title; ?></a>
...
-      <p class="search-snippet"><?php print $snippet; ?></p>
+      <p class="search-snippet"<?php if ($result_language) : ?> xml:lang="<?php print $result_language; ?>"<?php endif; ?>><?php print $snippet; ?></p>

um, why don't we use a $attributes and/or $class_array/$class variable like everywhere else here?

Powered by Dreditor.

mgifford’s picture

mgifford
he/him
Primary language English
commented

Where do we use $attributes and/or $class_array/$class variable in this template? I looked at this a bunch, but these are the only variables I see available:

* Available variables:
* - $url: URL of the result.
* - $title: Title of the result.
* - $snippet: A small preview of the result. Does not apply to user searches.
* - $info: String of all the meta information ready for print. Does not apply
* to user searches.
* - $info_split: Contains same data as $info, split into a keyed array.
* - $module: The machine-readable name of the module (tab) being searched, such
* as "node" or "user".

It's not in template_preprocess_search_result() either that I can see.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented

In contrib nodes will be able to hold content in multiple languages through translatable fields so we'll have to rethink this approach, but for core this is the right solution. Except for sun's remark this looks good and seems ready to go.

mgifford’s picture

mgifford
he/him
Primary language English
commented

@plach I'm still not sure how to resolve @sun's remarks. @sun will likely get to it soon enough, but any ideas would be great!

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Sorry if this didn't come across: I meant that we are using $attributes and/or $class_array/$class variables in all other templates, exactly for this purpose, but suddenly, we are not using them here. We should be using the same variables here. You can basically look into most other template_(pre)process_* functions to figure out how they are built and used.

mgifford’s picture

mgifford
he/him
Primary language English
commented
Status: Needs work » Needs review

#16: 867114-langnone.patch queued for re-testing.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

@sun: I am not following your comments in #31/#27, or else I think they don't apply to this situation, if I understand what you are saying.

I think where we are using $class/$class_array/$attributes in other templates, the output is a single HTML tag, and we are supplying the class or other attributes for that single HTML tag.

In this case, though, the output is

<li><h3><a (attribute goes here)>link text</a></h3><div><p (attribute goes here)>content</p><p>meta info</p></div></li>

So I am not sure that using the standard $class/$attributes usage in other templates is really all that similar, since this is a specific attribute that goes only on two inner tags of the output?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

We need to get this in before release, or it's going to get bumped to D8, I think.

sun, care to answer #33 and explain what you think we need to do to make this acceptable, or make a new patch?

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
StatusFileSize
new drupal.search-result-language.35.patch5.33 KB

Consistency. All other templates are done this way.

For example, RDF doesn't plug into search results yet, and this is the only way to do that.

mgifford’s picture

mgifford
he/him
Primary language English
commented

I've applied it to an updated sandbox.

Looks pretty good. What needs to happen to make this rtbc?

BarisW’s picture

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

What needs to happen to make this rtbc?

Marking it as such :)

I've tested it on a clean 7 dev, and it looks great. I tried with Dutch, English and Undefined nodes.
- In the Dutch interface, only the English nodes get a 'lang' attribute
- In the English interface, only the Dutch nodes get a 'lang' attribute
- Undefined nodes never get a 'lang' attribute, which is good

So basically the same output as #16, but now with a better architecture.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Assigned: jhodgdon » Unassigned

Thanks sun.

dries’s picture

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

Committed

Status: Fixed » Closed (fixed)

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