Last Comment Bug 208601 - Services are non-functional in form fields
: Services are non-functional in form fields
Status: RESOLVED FIXED
: fixed1.8, regression
Product: Camino Graveyard
Classification: Graveyard
Component: OS Integration (show other bugs)
: unspecified
: PowerPC Mac OS X
: P3 normal with 1 vote (vote)
: Camino1.0
Assigned To: Simon Fraser
: Chris Petersen
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2003-06-06 23:16 PDT by ender_mb
Modified: 2005-09-04 13:54 PDT (History)
5 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Patch to expose and use nsIPresShell::GetSelectionForCopy() (7.01 KB, patch)
2005-08-27 22:54 PDT, Simon Fraser
roc: review+
roc: superreview+
asa: approval1.8b4+
Details | Diff | Review

Description ender_mb 2003-06-06 23:16:29 PDT
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 2003-06-08 10:40:05 PDT
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 2003-06-08 11:58:06 PDT
BTW, this used to work in 0.7.
Comment 3 Thilo Planz 2003-07-08 21:17:17 PDT
I am running 10.2.6 and this bug affects Jaguar as well.
Comment 4 Chris Casciano 2004-12-14 19:29:35 PST
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 2004-12-14 20:01:51 PST
(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 2004-12-15 08:07:08 PST
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) 2005-07-29 23:21:12 PDT
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) 2005-07-30 00:27:35 PDT
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) 2005-08-12 12:35:45 PDT
(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 2005-08-24 09:48:10 PDT
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 2005-08-24 09:51:47 PDT
Or I could just expose PresShell::GetSelectionForCopy() in nsIPresShell.
Comment 12 Boris Zbarsky [:bz] (Out June 25-July 6) 2005-08-24 17:18:27 PDT
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) (Exited; email my personal email if necessary) 2005-08-24 18:09:17 PDT
Neither am I. If you can't find anyone who is, I'll look at the code.
Comment 14 Simon Fraser 2005-08-24 22:58:48 PDT
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) (Exited; email my personal email if necessary) 2005-08-25 10:30:07 PDT
(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 2005-08-27 22:54:22 PDT
Created attachment 194079 [details] [diff] [review]
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 2005-08-29 08:12:19 PDT
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 2005-08-29 08:15:34 PDT
Checked into trunk.
Comment 19 Asa Dotzler [:asa] 2005-09-04 10:09:03 PDT
Please get this verified on the trunk and then landed on the branch. Time is
short for beta.
Comment 20 Simon Fraser 2005-09-04 13:54:02 PDT
Checked in on the branch.

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