bookmark keywords ending in space or containing spaces aren't recognized when used with search terms (smart keywords)

RESOLVED FIXED in Firefox 2 beta1

Status

()

--
minor
RESOLVED FIXED
16 years ago
13 years ago

People

(Reporter: joachims, Assigned: ispiked)

Tracking

({fixed1.8.1})

2.0 Branch
Firefox 2 beta1
fixed1.8.1
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

16 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5a) Gecko/20030728 Mozilla Firebird/0.6.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.5a) Gecko/20030728 Mozilla Firebird/0.6.1

When entering bookmark keywords (also called keymarks by Eric A. Meyer in the
devedge link http://devedge.netscape.com/viewsource/2002/bookmarks/ ) an
accidental space at the end causes the keyword to not be recognized when used in
the location bar with a search term to fill the %s of the URL. Instead the words
get used in a google "I'm Feeling Lucky" search as if there were no keyword
present.  

The keyword by itself (without a search term) seems to work fine to load a
bookmark even if it has a space.

Reproducible: Always

Steps to Reproduce:
1. Setup a bookmark with a %s in the URL.
2. Set a keyword that ends in a space
3. Use the keyword with search terms in the location bar to attempt to access
the bookmark and fill the %s.

Actual Results:  
Google "I'm Feeling Lucky" search occurs

Expected Results:  
Mozilla (Firebird) should ignore the space at the end of the keyword when
matching keyword and filling in the search terms in the %s of the URL. 
Additionally, keywords (keymark) containing a space in the middle should also
work (a keyword such as "search this").
okay, so I think I have this one tested out fully.  Since the string set is "foo
" not "foo", using "foo" won't find a match in keywords and will proceed to the
URL search terms.  "search this" as a simple bookmark keyword works fine, but
breaks on search bookmarks, due to how the arguments get called. "search " works
as a search bookmark using the syntax "search  foo" but not with "search foo"
due to there not being a space between the keyword string and the search
arguments (since the first space is part of the string).

The simple solution is to prevent spaces from being entered in keywords,
although this would exclude a simple bookmark keyword such as "my bugs" from
being used for non-search bookmarks, however the inconsistency is something that
should be avoided.  Preventing people from getting any impression, however
misguided, that Firebird is broken is a good thing.
Status: UNCONFIRMED → NEW
Ever confirmed: true
we could prevent use of spaces directly in the properties window
by doing:
document.getElementById("shortcut").value =
document.getElementById("shortcut").value.split(" ").join("");
via the onkeyup event in the keyword field.

or we could just transparently strip them out (possibly confusing people trying
to set multiple word keywords, but should they even do this?)

      if (newvalue && gProperties[i] == (NC_NS + "ShortcutURL")) {
        // shortcuts are always lowercased internally
        newvalue = newvalue.toLowerCase();
+       // strip spaces
+       newvalue = newvalue.split(" ").join("");
      }

personally, I like the second option better, since in the most part, people
won't think about or try to use multiple word keywords.  Those who do will
probably go back in and see the spaces removed and figure it out.
(Reporter)

Comment 3

16 years ago
Of the two options you give, I think it is better to prevent spaces from being
entered at all in the keywords.  Transparently deleting spaces for those that
intentionally enter them to create a multi-word keyword will just frustrate
those users because they know what they thought they did and Firebird changes it
on them without informing them. Preventing spaces from being entered gives no
such illusion but could be frustrating for those who don't understand why their
spacebar doesn't work. :)

I still think that the ideal is:
1. Transparently trim any trailing spaces from the keyword. This solves the
hanging space problem.
2. Consider changing how search bookmarks get called to allow multi-word
keywords to be set with spaces to correctly pass the URL search terms after the
multi-word keyword.
*** Bug 274634 has been marked as a duplicate of this bug. ***
--> mconnor, per comment in bug 274634
Assignee: p_ch → mconnor
blocking entry of spaces seems to make more sense right now, will clean this up
for 1.1.
Severity: normal → minor
Status: NEW → ASSIGNED
Target Milestone: --- → Firefox1.1
*** Bug 304415 has been marked as a duplicate of this bug. ***
OS: Windows 2000 → All
Hardware: PC → All
Version: unspecified → Trunk
Summary: bookmark keywords ending in space or containing spaces aren't recognized when used with search terms → bookmark keywords ending in space or containing spaces aren't recognized when used with search terms (smart bookmarks)
Summary: bookmark keywords ending in space or containing spaces aren't recognized when used with search terms (smart bookmarks) → bookmark keywords ending in space or containing spaces aren't recognized when used with search terms (smart keywords)
Assignee: mconnor → nobody
Status: ASSIGNED → NEW
QA Contact: mconnor → bookmarks
(Assignee)

Comment 8

13 years ago
Created attachment 225788 [details] [diff] [review]
el patch

Shouts go out to mconnor for giving me something easy to search LXR for to and to logan for help with the regexp.
Assignee: nobody → ispiked
Status: NEW → ASSIGNED
Attachment #225788 - Flags: review?
Attachment #225788 - Flags: approval-branch-1.8.1?
(Assignee)

Updated

13 years ago
Attachment #225788 - Flags: review?(mconnor)
Attachment #225788 - Flags: review?
Attachment #225788 - Flags: approval-branch-1.8.1?(mconnor)
Attachment #225788 - Flags: approval-branch-1.8.1?
Attachment #225788 - Flags: review?(mconnor)
Attachment #225788 - Flags: review+
Attachment #225788 - Flags: approval-branch-1.8.1?(mconnor)
Attachment #225788 - Flags: approval-branch-1.8.1+
(Assignee)

Comment 9

13 years ago
Filed bug 341795 for doing this for Places.
Whiteboard: [checkin needed]
Target Milestone: Firefox1.5 → Firefox 2 beta1
Version: Trunk → 2.0 Branch
mozilla/browser/components/bookmarks/content/bookmarksProperties.js 	1.25
mozilla/browser/components/bookmarks/content/bookmarksProperties.js 	1.22.4.3
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Whiteboard: [checkin needed]
You need to log in before you can comment on or make changes to this bug.