Focus Events not getting past editor

VERIFIED FIXED in mozilla0.9.1

Status

()

Core
Editor
VERIFIED FIXED
17 years ago
17 years ago

People

(Reporter: John Gaunt (redfive), Assigned: John Gaunt (redfive))

Tracking

Trunk
mozilla0.9.1
x86
Windows NT
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

(Assignee)

Description

17 years ago
For Accessibility we need to catch all the focus events to tell MSAA when focus
changes. Ender uses focus to activate the caret, but then sets the event to not
bubble any more to avoid passing a focus event to the web page itself.

I tried commenting out the PreventBubble() call but that has the side effect of
focusing the browser itself. Planning to add code to fire a custom event for
which only the nsRootAccessible will be listening as I have done in
content/html/content/src/nsHTMLInputElement.cpp ( in the
Accessible_042501_Branch3 branch ).
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
(Assignee)

Comment 1

17 years ago
Hmm... The code I wanted to add to nsEditorEventListeners.cpp needing to get to
the PresContext and needed a ListenerManager and I need to set a target... It
was getting pretty ugly. mjudge I need to talk with you at some point about
getting that in.

In the meantime I can work that code into the individaul HTML element classes.
(Assignee)

Comment 2

17 years ago
I have added code to generate custom events for the text areas in the specific
content files for text areas, password fields etc... The goal is to move this
code into the event handler mentioned above.

pushing this bug out to 0.9.2 and dropping the severity since there is a sort of
work around in the tree.
Severity: critical → normal
Target Milestone: --- → mozilla0.9.2
(Assignee)

Comment 3

17 years ago
Created attachment 34763 [details] [diff] [review]
Patch for focus events
(Assignee)

Comment 4

17 years ago
Patch added to change us to a capturer of events for Focus ( and for form 
change) events. This fixes the problem of us not getting the focus events for 
textAreas and text input fields due to Ender consuming them.

I have also removed some listener interfaces we don't need right now.

aaron, eric
looking for r=
Whiteboard: need r=
Target Milestone: mozilla0.9.2 → mozilla0.9.1

Comment 5

17 years ago
r=aaronl
(Assignee)

Updated

17 years ago
Summary: Focus Events note getting past editor → Focus Events not getting past editor
Whiteboard: need r= → have r= need sr=
+      // capture DOM focus events 
+      rv = target->AddEventListener( nsAutoString(NS_LITERAL_STRING("focus")) ,
listener, PR_TRUE );
+      NS_ASSERTION(NS_SUCCEEDED(rv), "failed to register listener");
+      // capture Form change events 
+      rv = target->AddEventListener( nsAutoString(NS_LITERAL_STRING("change"))
, listener, PR_TRUE );
+      NS_ASSERTION(NS_SUCCEEDED(rv), "failed to register listener");

Don't uselessly copy into an nsAutoSTring implicit temporary, do use
nsLocalString(NS_LITERAL_STRING("...")) to satisfy the const nsAReadableString&
first parameter mapped from IDL 'in domstring'.

Oddly indented comments here:

+NS_IMETHODIMP nsRootAccessible::Select(nsIDOMEvent* aEvent) { return NS_OK; }
 
-NS_IMETHODIMP nsRootAccessible::Input(nsIDOMEvent* aEvent)
-{
-  // get Input events when text is entered or deleted in a textarea
+  // get Input events when text is entered or deleted in a textarea or input
   //return HandleEvent(aEvent);
- ... [lots of deleted lines]
+NS_IMETHODIMP nsRootAccessible::Input(nsIDOMEvent* aEvent) { return NS_OK; }

The next line after the commented-out return is a one-line definition for Input,
so what does the comment really mean, and to which code does it apply?

/be
(Assignee)

Comment 7

17 years ago
Created attachment 34848 [details] [diff] [review]
new patch, changing nsAutoString to nsLocalString
(Assignee)

Comment 8

17 years ago
The comments are basically some documentation so that when we get to 
implementing that functionality I don't have to go back and re-test to see which 
function handles exactly which behavior/widget. It's very hard to read in the 
diff, makes more sense looking at the file, I'll attach that section here.
(Assignee)

Comment 9

17 years ago
Created attachment 34850 [details]
clarification of the last section of code
(Assignee)

Comment 10

17 years ago
Created attachment 34874 [details] [diff] [review]
a patch that acutally compiles on linux
(Assignee)

Comment 11

17 years ago
That last patch takes out the nsLocalString part -- that doesn't compile on 
Linux, complains about not being able to find a matching function call. So we're 
down to a NS_LITERAL_STRING("..") call, which I swear I tested and it didn't 
work right, but now I try it and it works just fine!!

Removing mjudge from cc
Cool, thanks for trying the even simpler thing, despite it probably not working
in the past!  Cc'ing scc; scott, did things change so that NS_LITERAL_STRING now
suffices as an actual argument matching a const nsAReadableString& formal param,
on all platform/compiler combos?

/be
sr=brendan@mozilla.org, forgot to record last time.

/be

Comment 14

17 years ago
An |NS_LITERAL_STRING| satisfies |const nsAString&| (and therefore its
|typedef|, |nsAReadableString|) on all platforms.  What it _doesn't_ do,
currently, is _fail_ to satisfy |NS_ConvertASCIItoUCS2|, or |nsAutoString| on
platforms where |L"..."| doesn't work.  Preventing people from using it in this
way is discussed in bug #63923.  There is a patch in progress there that you
might be interested in perusing.
(Assignee)

Comment 15

17 years ago
fix checked in 2001.05.18
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
(Assignee)

Comment 16

17 years ago
slight regression due to checkin for bug 80505. Changed the way we get an
Accessible object. Instead of using QI we now call GetAccessible on a frame. The
case in nsRootAccessible wasn't taken care of.

-- am also seeing weird behavior in the QI to nsIContent from the target, first
time it never works, but after that everything is fine... investigating,

have a fix for the QI problem, will post for r=/sr=
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 17

17 years ago
Created attachment 35520 [details] [diff] [review]
One line diff QI -> GetAccessible
(Assignee)

Comment 18

17 years ago
Just posted a patch, looking for r=/sr= to try and get this in before the tree 
closes.
Whiteboard: have r= need sr= → regressed - need r=/sr= for 1(2) line patch

Comment 19

17 years ago
r=dr on the one-liner patch (attachment 35520 [details] [diff] [review]).
(Assignee)

Comment 20

17 years ago
adding brendan to cc list for visibility
Whiteboard: regressed - need r=/sr= for 1(2) line patch → regressed -- need sr= for 1(2) line patch

Comment 21

17 years ago
sr=scc
Sorry, my laptop was dying (bad SIMM) and I was out of touch -- thanks to scc
for helping.

/be
(Assignee)

Comment 23

17 years ago
checked in last night, thanks scc, brendan
Status: REOPENED → RESOLVED
Last Resolved: 17 years ago17 years ago
Resolution: --- → FIXED
Whiteboard: regressed -- need sr= for 1(2) line patch

Comment 24

17 years ago
John, can you verify if this is fixed, and also mark the bug VERIFIED-FIXED?

thanks.
(Assignee)

Comment 25

17 years ago
Verified 2001.05.30

focus events are getting through to the nsRootAccessible object for text fields.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.