208601 - Services are non-functional in form fields

Status: RESOLVED FIXED

(Camino Graveyard :: OS Integration, defect, P3)

Description

[ender_mb]

User-Agent:       Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.5a) Gecko/20030530 Camino/0.7+
Build Identifier: Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.5a) Gecko/20030530 Camino/0.7+

The summary says it all. Services (open selection in TextEdit, etc.) work
properly in the browser view, but are disabled in form controls (i.e. textboxes).

Reproducible: Always

Steps to Reproduce:
1. go to, say, the search field at the bottom of this page
2. type some text and highlight it
3. go to Camino>Services

Actual Results:  
all services are disabled

Expected Results:  
services menu options should be enabled

I'm running OS 10.1.5. I don't know if this affects Jaguar systems though.

Comment 1

[Derek Schrock]

Confirmed with Camino 2003060804 (this also happens with Mozilla). Also tested
in Safari to make sure other apps have this behavior.

Don't see any dups, in Camino or Mozilla reports.

Marking new, change any attributes if need be.

Comment 2

[ender_mb]

BTW, this used to work in 0.7.

Comment 3

[Thilo Planz]

I am running 10.2.6 and this bug affects Jaguar as well.

Comment 4

[Chris Casciano]

10.3.6/recent nightly

a few quick tests off highlighting a porting of this page's URL shows that
"search in google" [safari], email->send to [mail.app], textedit-> new window
with current selection all worked properly (when the recieving app was already
open, as is the case in other apps).


Is the total breakage specific to pre-10.3.x?

Comment 5

[ender.mb]

(In reply to comment #4)
> 10.3.6/recent nightly
> 
> a few quick tests off highlighting a porting of this page's URL shows that
> "search in google" [safari], email->send to [mail.app], textedit-> new window
> with current selection all worked properly (when the recieving app was already
> open, as is the case in other apps).
> 
> 
> Is the total breakage specific to pre-10.3.x?

This bug is about html form widgets in the page itself, not the url bar. the url
bar is a native text box and will automatically respond to services.

Comment 6

[Chris Casciano]

gotcha.. yes thats what i see...

its as if the menu doesn't realize there's a selection made.

Comment 7

[Samuel Sidler (old account; do not CC)]

Still broken on 2005072608. Not sure this meets 1.0 criteria though (whatever
that may be). Is the number of people that use the services menu (or would use
it) great enough that this should be fixed by 1.0?

Comment 8

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

As noted in comment 2, this is a regression between 0.7 and 0.7+ (20030530).

We see posts about this periodically in the forums, and it is a long-standing
regression; it's also an inconsistency because services work everywhere else in
Camino.

Depending on what it would take to fix this, it might be something to leave on
the list and then bump at the end if it doesn't make it.

Comment 9

[Paul Borokhov (lensovet)]

(In reply to comment #7)
> Still broken on 2005072608. Not sure this meets 1.0 criteria though (whatever
> that may be). Is the number of people that use the services menu (or would use
> it) great enough that this should be fixed by 1.0?

Enough times I've had this happen that it's rather annoying. Of course through
experience I've stopped reaching for the Services menu and just do copy/paste,
but esp since this is a reg, this would make a lot of sense to be done before 1.0

Comment 10

[Simon Fraser [no longer active]]

This is broken because DocumentViewerImpl::GetCanGetContents() is looking at the
wrong selection (that for the document, rather than the text field) and
concluding that the selection is collapsed.

I see a couple of ways to fix this:
  Add a method to nsIPresShell called CanGetContents() that grabs the correct
  selection (since nsIPresShell::DoGetContents() already does this)
or
  add an implementation of cmd_getContents to the editor, which will get it
  first and do the right thing.

Comment 11

[Simon Fraser [no longer active]]

Or I could just expose PresShell::GetSelectionForCopy() in nsIPresShell.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Simon, I'm not really familiar enough with the selection/editor setup to judge
which of those approaches is better....

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Neither am I. If you can't find anyone who is, I'll look at the code.

Comment 14

[Simon Fraser [no longer active]]

DocumentViewerImpl::GetCopyable() is broken when the focus is in a textarea
(since it only looks at the selection of the document, not the (different)
selection of the text input. So we have to get the correct selection based on
focus, which is what PresShell::GetSelectionForCopy() does.

So I have have to have the docViewer call presShell->GetSelectionForCopy(), or I
need to add nsIPresShell::CanCopy() or somesuch. It seems that exposing
GetSelectionForCopy() in nsIPresShell is the best solution.

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #14)
> So I have have to have the docViewer call presShell->GetSelectionForCopy(), or I
> need to add nsIPresShell::CanCopy() or somesuch. It seems that exposing
> GetSelectionForCopy() in nsIPresShell is the best solution.

I agree. Make it so!

Comment 16

[Simon Fraser [no longer active]]

Attachment: Patch to expose and use nsIPresShell::GetSelectionForCopy()

This patch exposes nsPresShell::GetSelectionForCopy() in nsIPresShell, and
changes GetCopyable() and GetCanGetContents() to use it (these methods do the
same thing). I added comments in a couple of other places that should probably
use it, but I want to keep the patch safe for the branch.

Comment 17

[Simon Fraser [no longer active]]

Comment on attachment 194079 [details] [diff] [review]
Patch to expose and use nsIPresShell::GetSelectionForCopy()

Requesting 1.8b4 approval. This is a regression, and Camino needs it. Camino is
the only user of this API, so it's a safe fix.

Comment 18

[Simon Fraser [no longer active]]

Checked into trunk.

Comment 19

[Asa Dotzler [:asa]]

Please get this verified on the trunk and then landed on the branch. Time is
short for beta.

Comment 20

[Simon Fraser [no longer active]]

Checked in on the branch.