In firefox, if you right-click on a form input, one of the context menu items is "Add a Keyword for this Search". It brings up the add bookmark dialog and you can enter a name and keyword. It uses the form's action to generate a bookmark.
This would lower the learning curve for creating the (rather useful) bookmark keywords since currently you have to perform a pretend search and then edit the generated URL, replacing the query with %s. The firefox feature also handles POST forms (assuming they can also take GET data).

Created attachment 501057[details][diff][review]
patch
Ported from current FF code. Really easy now that we use Places for bookmarks. :-)
Please put this after Sync and before final beta on your priority list.

(In reply to comment #5)
> See bug 570266,
OK, so form.action is weird. Should we also check the current element in case it has a formAction, see bug 607145?
> bug 359809,
Interesting... I'd have to wade through the form submission C++ code, which presumably gets this right. But the patch is still wrong, since it needs to convert from unicode before escaping.
> and bug 620565
OK, so this is a known bug.

(In reply to comment #7)
> Haven't looked at 570266 but 264607 was solved with XPCNativeWrappers (except
> for a silly bug where the action property is blank when the attribute is, when
> it should instead be the document's URI).
IIUC that means I can in fact use form.action. How about this:
var node = document.popupNode;
var doc = node.ownerDocument;
var charset = doc.characterSet;
var form = node.form;
var spec = form.action || makeURI(doc.URL, charset).spec;
(In reply to comment #8)
> (In reply to comment #5)
> > See bug 570266,
> OK, so form.action is weird. Should we also check the current element in case
> it has a formAction, see bug 607145?
I don't think that makes much sense. AFAIU formAction is mainly used for submit buttons but we only offer adding a keyword for text boxes.

(In reply to comment #9)
> var spec = form.action || makeURI(doc.URL, charset).spec;
Not sure what the effect of charset is here but the spec always seems to be the same as the doc.URL (or better still doc.documentURI in nsIDOM3Document).

(In reply to comment #10)
> Not sure what the effect of charset is here but the spec always seems to be the
> same as the doc.URL (or better still doc.documentURI in nsIDOM3Document).
See originCharset on https://developer.mozilla.org/en/nsIURI#Attributes, if that helps. Unfortunately I don't know enough about fancy charset/URI combinations, I never came across anything but ASCII (including urlencoded) which is contained in UTF-8 (completely?). And punycode is ASCII, too, but I don't know whether doc.URL/doc.documentURI would be punycode or something else. Maybe Ratty or someone at Mozilla knows more about that topic or could create a test case. Otherwise I think the makeURI/charset approach should be safe. In any case we should choose something here and maybe leave that issue for a follow-up if need be.

Comment on attachment 501295[details][diff][review]
patch v2
>--- a/suite/browser/navigator.js
>+++ b/suite/browser/navigator.js
Bah, context menus in text fields in mailnews are already broken because it fails to check spelling, so I can't tell whether this does something reasonable (e.g. just ignoring it because the item is never available).
>+ type = el.type.toLowerCase();
IIRC .type is already lower case

(In reply to comment #11)
> (In reply to comment #10)
> > Not sure what the effect of charset is here but the spec always seems to be the
> > same as the doc.URL (or better still doc.documentURI in nsIDOM3Document).
> See originCharset on https://developer.mozilla.org/en/nsIURI#Attributes, if
> that helps. Unfortunately I don't know enough about fancy charset/URI
> combinations, I never came across anything but ASCII (including urlencoded)
> which is contained in UTF-8 (completely?). And punycode is ASCII, too, but I
> don't know whether doc.URL/doc.documentURI would be punycode or something else.
It's not, I tried. But for fancy characters in the path, they're always UTF-8 encoded by the time they each doc.documentURI, even on Windows-1252 pages. So we can just use || doc.documentURI (which, unlike document.location, also doesn't have a #, so that's one less thing to think about.)
I believe the character set might make a difference for POST data, but we'll cross that bridge when we get to it.

(In reply to comment #12)
> Bah, context menus in text fields in mailnews are already broken because it
> fails to check spelling
Such things don't tend to fix themselves, so I filed bug 623590 for you.
> so I can't tell whether this does something reasonable
> (e.g. just ignoring it because the item is never available).
If I comment out the lines that break the MailNews context menu, it appears normally, and produces no error messages related to the new code (as you said, the item is never available since it's defined in the same file where the item the absence of which breaks the context menu is defined).
> >+ type = el.type.toLowerCase();
> IIRC .type is already lower case
So it seems, and allows us to get rid of the temp variable altogether (s/type/el.type/).
(In reply to comment #13)
> So we can just use || doc.documentURI (which, unlike document.location, also
> doesn't have a #, so that's one less thing to think about.)
Applied locally. Anything issues left open? Want a new patch?

(In reply to comment #14)
> (In reply to comment #12)
> > >+ type = el.type.toLowerCase();
> > IIRC .type is already lower case
> So it seems, and allows us to get rid of the temp variable altogether
Well, it does get used several times...
> (In reply to comment #13)
> > So we can just use || doc.documentURI (which, unlike document.location, also
> > doesn't have a #, so that's one less thing to think about.)
> Applied locally. Anything issues left open? Want a new patch?
Only for posterity.

(In reply to comment #18)
> You also added a bunch of trailing spaces :-(
Yes, in two otherwise empty lines. Sorry for that. I blame our policy, though (at work I can let my editor program remove all trailing white space, for Mozilla-related stuff I have to maintain exceptions just so that the existing ugly code stays blame-friendly). Anyway, I'm digressing. If you like I can push another white space fix (same bug and similar time, so should be blame-friendly).

Note

You need to
log in
before you can comment on or make changes to this bug.