split from http://drupal.org/node/146466#comment-326591

If you do a search [term1 AND term2] you get results for "term1", "and" and "term2". To get [term1 AND term2] behaviour you actually need to put [term1 term2] in the search box because AND is assumed, and has no special treatment.

So there should probably be a small function in the search query evaluation that looks for uppercase AND and strips it from queries, ["George and Milda"] would still work as a phrase search, but [bananas AND apples] would be stripped to [bananas apples] before processing and therefore resemble most other search interfaces.

As well as making it a bit more standard, it should also be a performance improvement [bananas AND apples] queries, and make them more accurate, since it'd be one less word to search and rank. I don't have the skills to roll a patch for this, just wanted to get this out of the other issue to avoid holding it up.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

greggles’s picture

What about looking for " and " in their search phrase and, if present, doing a drupal_set_message warning them that and is implied and not necessary and that they may want to repeat the search without the word?

I'm hesitant to simply remove the word from their search - what if you really want to search for "and"?

catch’s picture

regular expressions can decide if something is lowercase or not, and whether it's in quotes though right? That ought to cover most use cases where someone really wants to search for and, rather than using AND.

douggreen’s picture

Category: bug » feature
FileSize
1.87 KB

This is a usability issue.

The warning message are "borrowed" from Google. In the original 6.x patch a warning was added for OR. This patch adds a warning for AND.

Here are the scenarios:

  1. "A or B" - the lower case "or" is a two character word which is ignored (by most peoples settings), a warning is displayed, and an AND query is performed. This was already handled by the previous 6.x search patch.
  2. "A OR B" - standard OR query, nothing new here.
  3. "A and B" - the lower case "and" is now ignored and a warning is displayed. This is a change in behavior added by this patch.
  4. "A AND B" - same as 3 above. the upper case "AND" is now ignored and a warning is displayed. This has never been valid search syntax for a query, and I think, the source of the usability confusion. Does this patch make things clearer?
douggreen’s picture

Status: Active » Needs review
catch’s picture

Status: Needs review » Reviewed & tested by the community

Yes, it does. I tested with the various options, and also double checked phrase searches don't get caught just in case. Everything worked fine, and the text makes sense.

douggreen’s picture

Status: Reviewed & tested by the community » Needs review

Thanks catch! So, ... we know that the patch works and that the wording pretty much works.

But, before we commit this as-is, ... I am uncomfortable that I "borrowed" the text from Google. It was really meant as a placeholder, as a very good basis to slightly tweak. But I'd hoped that someone else would offer a few small word tweaks that made it our own. Since this patch has my name on it, I'd really prefer that we change this before distributing it to the world. I don't want to be known as a plagiarist.

catch’s picture

Ah, I didn't realise you'd taken it wholesale, should've checked on google I guess.

In that case how's this?

- Try uppercase "OR" to search for either of two terms.
+ Use OR (uppercase) to search for posts containing either of two terms

- The "AND" operator is unnecessary -- we include all search terms by default.
+ All search terms are included by default, so the word "and" has been removed from your query. "And" will not be stripped as part of a phrase search, i.e. "It was raining cats and dogs".

The second one's possibly too long.

Bevan’s picture

- Try uppercase "OR" to search for either of two terms.
+ Search either of the two terms with uppercase OR.
+ e.g. cats OR dogs

- The "AND" operator is unnecessary -- we include all search terms by default.
+ AND is not necessary. All terms are included by default.
+ e.g. cats AND dogs is the same as cats dogs.

It would be good if the examples could include terms from the users search.

wmostrey’s picture

I like the OR text, but I'm not sure the part about the query should be included in the AND text, or if there should even be an AND text:

All search terms are included by default. Use OR (uppercase) to search for posts containing either of two terms.

Bevan’s picture

@wmostrey I wondered that too. How about something more subtle like

- The "AND" operator is unnecessary -- we include all search terms by default.
+ By the way, cats AND dogs is the same as cats dogs.

Where the user searched for cats AND dogs or cats and dogs. And the terms either side of the first 'AND' in the search request are substituted in for 'cats' and 'dogs' in that text.

douggreen’s picture

What if the user searches for "miniature dachshund" AND sheepdog AND "Australian blue hound", do you still think we should substitute these terms in the help? That's adding some complicated logic to a simple help message for Drupal core.

I'd be up for substituting the terms as long as it's a simple query, and the code turns out to be simple, but I won't know until I write it.

akanowicz’s picture

Bevan- try to avoid colloquialisms like "By the way"

- Try uppercase "OR" to search for either of two terms.
+ A search using OR will return results that contain either term

- The "AND" operator is unnecessary -- we include all search terms by default.
+ A search using multiple terms will include all terms by default. Using "AND" is not necessary.

Bevan’s picture

@douggreen

the terms either side of the first 'AND' in the search request are...

So for "miniature dachshund" AND sheepdog AND "Australian blue hound" the message would be

Did you know a search for "miniature dachshund" AND sheepdog is the same as a search for "miniature dachshund" sheepdog?

@akanowicz Good point. Thank you. I'm not sure what to use here. Do you think the above is an improvement?

I'm not sure if this is approach is even an improvement on simply

AND is not necessary. All terms are included by default.
wmostrey’s picture

I'm really not sure if we should include any message like "AND is not necessary" or ""miniature dachshund" AND sheepdog is the same as a search for "miniature dachshund" sheepdog". If it's the same, and people are used to putting "AND" in there, or feel it's more intuitive, why not just let them? I feel this is more of a behind-the-scene thing that should not necessarily be communicated to the user, this might cause confusing. I still feel the following is the best and most simple approach:

All search terms are included by default. Use OR (uppercase) to search for posts containing any of the terms.

catch’s picture

I'm leaning towards not saying anything about AND as well.

douggreen’s picture

FileSize
1.83 KB

The attached patch implements the following:

  • OR - The OR wording still open for refinement. Here's my suggestion Search for either of the two terms with an uppercase "OR"<br/>for example, %cats OR %dogs. (mostly from Bevan), where %cats and %dogs is replaced by the first two search terms. I like akanowicz suggestion but it feels like it's missing a phrase before such as If these results aren't what you expected, ....
  • AND - consensus seems to be drop the warning. This patch still implements a change in behavior, though, which is to strip out the keyword "and" (uppercase and lowercase), and just return search results as-if the word was not used. I suspect that this is the desired behavior. If someone wants to search for the word "and" they can put it in quotes as a phrase and it should still search for it (I think).

I have two technical concerns.

  • One problem with embedding the search words in the message is that if one of the terms is a phrase, the warning message does not include the quotes. Adding the quotes would add lines of code to the warning logic that I'd prefer to avoid. Try searching for "miniature dachshund" or sheepdog and you'll see what I mean.
  • The new warning has an t('....<br/>...'). I know that we frequently embed <a href> inside t(), is there any reason to not embed the <br/>? Is the <br/> needed? Bevan's original example had it, and I do think it makes it easier to read.
catch’s picture

One problem with embedding the search words in the message is that if one of the terms is a phrase, the warning message does not include the quotes. Adding the quotes would add lines of code to the warning logic that I'd prefer to avoid. Try searching for "miniature dachshund" or sheepdog and you'll see what I mean.

I think I'd prefer "cats OR dogs" with no string replacement than the extra logic. Stripping the quotes in the helper message is likely to cause more problems than having fixed examples.

douggreen’s picture

Assigned: Unassigned » douggreen
FileSize
1.77 KB

There are more substitution issues involving other edge cases, such as if the user searched for cats dogs or birds the current implementation would substitute "cats" for "cats", but would substitute "or" for "dogs".

So, this patch just implements for example, cats OR dogs with no substitutions.

Once everyone's in agreement on the wording, please apply and make sure I didn't muck up the php (I did the last edit in the patch file without testing). Thanks!

catch’s picture

Status: Needs review » Reviewed & tested by the community

Patch applies cleanly and works well. I think the wording is fine. My main reason for opening this issue was the possibility of bad results when 'and' was included, that's more than dealt with. So this should be ready.

keith.smith’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
30.79 KB

Slightly Off Topic: If the search returns no results, some search hints are already displayed (see attached screenshot), including a reference to "loosening your query with OR". I hate to widen this issue, but I'd love to take any opportunity to lobby for an example other than "blue smurf". Not because I am particularly Gargamel-like, I've just always thought the Department of Redundancy Department should be alerted about that example. Though there is much in this article I did not know, it assures me that smurfs are, by nature, blue.

Edit: I cross-posted with the RTBC, and didn't mean to change status. Please set this back to RTBC if I should take up my smurf crusade in another issue (and more importantly, these two different examples do not seem to conflict).

catch’s picture

Status: Needs review » Reviewed & tested by the community

I think the help text is a new issue, definitely scope for improvement there though including 'blue smurf'.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Needs work

I don't think that <br/> looks good at all, neither on the screenshot, neither in the code. Otherwise as far as I see, this patch looks fine.

Bevan’s picture

FileSize
4.66 KB
27.31 KB
2.52 KB

I added appropriate markup and one css selector to distinguish the quoted terms and the example.

I don't think they need to be on two lines though. Actually I think they look better on one line (see the screenshot).

The user hasn't made a mistake by typing or -- it's not an error. So I changed form_set_error() for drupal_set_message(). drupal_set_message isn't right though either. Can we make this a tip on the search field, like in the other attached image?

AND - consensus seems to be drop the warning.

I agree that this makes sense.

douggreen’s picture

If you use drupal_set_message instead of form_set_error, you lose the red highlight around the form keyword input box. Personally I would probably stick with form_set_error.

I think the other changes look good onscreen.

But this is not how core does things. For example, you can grep the code from the module directory $ grep 't(' */*.module | grep '<div' (also do this with strong). You'll see some precedence for strong, but not for div. I'm wondering if we'll need to break this up as in the following example, and if the reason to do this is because of string translation?

t('Search for either of the two terms with uppercase <strong>OR</strong>.') .'<div class="search-tips-example">'. t('For example, <strong>cats OR dogs</strong>') .'</div>'

Or can we just do it the way Bevan has it.

Bevan’s picture

Ah yes.

should be outside the t().

If you use drupal_set_message instead of form_set_error, you lose the red highlight around the form keyword input box.

Exactly. That's why drupal_set_message is better IMHO :)

Gábor Hojtsy’s picture

Hm, why complicate that with this div at all?

catch’s picture

Status: Needs work » Needs review
FileSize
2.01 KB

Here it is minus the div and css.

douggreen’s picture

Status: Needs review » Needs work

@bevan, I'm referring to the red border around the form input field, not the big red box around the error. I don't like losing that visual clue. I'd prefer to keep the form_set_error.

Exactly. That's why drupal_set_message is better IMHO :)

I think that there might actually be another issue that could be opened (and I think it's too late to do anything for 6.x), to add an argument to form_set_error, that would pass-through to drupal_set_message(), so that you could better control the look-and-feel of the form errors (i.e., a form warning, form status). But I haven't done any searching to see if such an issue exists.

Also, there's no reason to set $message and then use it in the next line, since $message is only used in one place. I'd combine these into one line.

Bevan’s picture

FileSize
127.61 KB

I'm referring to the red border around the form input field

I understand. From a usability POV, entering 'or' and not 'OR' isn't an error IMHO. It's more of a 'FYI' for the user. The appearance of form_set_error() leads the user to believe that their search is not valid and they made a mistake. However their search including 'or' IS in fact a valid search. The system is just checking if they meant 'OR' instead of 'or'.

Hm, why complicate that with this div at all?

Also, there's no reason to set $message and then use it in the next line, since $message is only used in one place. I'd combine these into one line.

I agree.

I think this is almost RTBC. The only change I suggest is if we can move the message into the tips for the search field. I don't know enough about the context or fapi to attempt this without spending more time than I can afford right now. See the screenshot to see what I mean.

I don't think any of the advanced search fields. would need this, right?

douggreen’s picture

Status: Needs work » Needs review
FileSize
1.94 KB

I think that Bevan has an interesting idea for moving where the error is displayed, but IMO, that's another issue. I don't want to hold this one up any more.

douggreen’s picture

Status: Needs review » Reviewed & tested by the community
Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Thanks, committed. If you find a clean way to move the tip to the form item description area though, let's get that solved too.

Anonymous’s picture

Status: Fixed » Closed (fixed)

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