Need mechanism that allows us to figure out how focus was gained

RESOLVED FIXED

Status

()

P2
normal
RESOLVED FIXED
18 years ago
9 years ago

People

(Reporter: kinmoz, Assigned: blizzard)

Tracking

Trunk
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 obsolete attachment)

(Reporter)

Description

18 years ago
This is an offshoot of bug #28583.

Right now there is no way for text widgets to tell if focus was gained as the 
result of the user tabbing between form widgets, or by any other means.

We need a mechanism in the event system that allows the textwidget to tell 
how/why it got focus. This is neccessary because on some platforms, tabbing into 
a textwidget is supposed to select all it's contents, and any other method of 
focus gain leaves the selection/caret in the textwidget where it was before.
(Reporter)

Updated

18 years ago
Blocks: 28583

Comment 1

17 years ago
QA contact updated
QA Contact: gerardok → madhur

Updated

17 years ago
Target Milestone: --- → mozilla0.9.3

Comment 2

17 years ago
Doesn't look like this is getting fixed before the freeze tomorrow night.
Pushing out a milestone.  Please correct if I'm mistaken.
Target Milestone: mozilla0.9.3 → mozilla0.9.4

Updated

17 years ago
Priority: -- → P2

Comment 3

17 years ago
->0.9.5
Target Milestone: mozilla0.9.4 → mozilla0.9.5

Comment 4

17 years ago
Moving to 0.9.6
Target Milestone: mozilla0.9.5 → mozilla0.9.6

Comment 5

17 years ago
CC'ing bryner, who is working on similar things.

Comment 6

17 years ago
0.9.6 has passed, moving to 0.9.7.  Load balancing 0.9.7 list shortly
Target Milestone: mozilla0.9.6 → mozilla0.9.7

Comment 7

17 years ago
Moving bugs from 0.9.7 for triaging in 0.9.8
Target Milestone: mozilla0.9.7 → mozilla0.9.8

Updated

17 years ago
Target Milestone: mozilla0.9.8 → mozilla0.9.9

Updated

17 years ago
Target Milestone: mozilla0.9.9 → mozilla1.0

Updated

17 years ago
Target Milestone: mozilla1.0 → mozilla1.0.1
*** Bug 127272 has been marked as a duplicate of this bug. ***
Nominating for nsbeta1, since it is blocking bug 28583 which is nsbeta1+.

Keywords: nsbeta1

Updated

17 years ago
Keywords: nsbeta1 → nsbeta1+

Comment 10

17 years ago
I'm going to fix this the right way, the way the title of the bug suggests.

I need the "how focused was gained" mechanism to fix bug 131139 as well.
Assignee: joki → aaronl

Comment 11

17 years ago
Does anyone see the need for more than a PRBool aFocusedWithMouse?
Status: NEW → ASSIGNED
Target Milestone: mozilla1.0.1 → mozilla1.0

Comment 12

17 years ago
-> Pete Zha.

We discussed the fix on IRC>
Assignee: aaronl → pete.zha
Status: ASSIGNED → NEW

Comment 13

17 years ago
Pete, I added dependencies for all the bugs this can easily help fix.
The fix for bug 132099 and 131139 is the same. In
nsEventStateManager::ChangeFocus(), we need to not call MoveCaretToFocus() if
focus was gained via the mouse.

One extra detail: if we use this method to suppress MoveCaretToFocus(), we need
to make sure we move from the focus location instead of the selection the next
time we call nsEventStateManager::ShiftFocus(). This is because the focus has
changed, but the selection could still be in a completely different place. An
exception to this is if the focus is on the document -- in that case we always
want to move from the selection location if we have a selection.

This probably sounds really hard - it's not. If you post a patch I'll help you
with these extra tweaks.
Blocks: 131139, 132099

Comment 14

17 years ago
focus gained by: Mouse, Keyboard, Application?

Is there some reason not to use an integer/enum?

Comment 15

17 years ago
We need to decide whether to add a parameter to nsIContent::SetFocus() or add an
additional method like nsIContent::SetFocusFrom()

Comment 16

17 years ago
I think maybe we don't need to add a new interface or change the old one...
We can add something to mPresContext to pass the focusWith flag.
Like add Get/SetFocusedWith function into nsIPresContext.h and add mFocusedWith 
to nsPresContext.h.
And when we ChangeFocus, we can pass the flag to nsPresContext first.
I will upload a patch for this solution later.

Comment 17

17 years ago
Created attachment 75559 [details] [diff] [review]
Use nsIPresContext to carry focused with flag

This is a workarround patch.
Just work for me. With this patch, I can know where the focus from. Like
Keyboard, Click or other application.

Comment 18

17 years ago
I recommend posting one patch that shows the fixes for bug 28583, bug 131139,
bug 132099 and this one, so we can see it and review it all together.

Also, in nsPresContext.cpp, why are you adding 
+NS_IMETHODIMP
+nsPresContext::GetUseFocusColors(PRBool& aUseFocusColors)

instead of ::SetFocusedWith()

Comment 19

17 years ago
More comments:

Let's use an enum, not an int. It doesn't make sense to go 1,2,4,8 unless the
focus can be gained several different ways at the same time. Therefore, use
0,1,2,3...

aFocusedWithMouse should be aFocusedWith, since it's not a bool anymore that
just relates to where it was focused with the mouse. It's holding the enum.

I spoke with Bryner. We feel it's not necessary to bloat pres context with this.
If anything, this should be a global/static value, since there's only 1 global
focus at a time. However, we feel it might be better to insert this information
in the event or change ::SetFocus(presContext) to add a how param.

CC'ing Chris Saari and Tom Pixley for some fresh perspective.   

Comment 20

17 years ago
It looks okay to fix 131139 and 132099 using the ChangeFocus modification, but
doing a more general solution is something I haven't wrapped my head around yet,
at least not a good way to fix it. The problem becomes nasty when you consider
that the event system finds out about focus events often from OS native events
which may or may not have come from our system (mouse down handling or tab or
script). So we get in this spiral of tracking event origination in a global
way... which is the exact sort of thing I'm trying to do less of in the focus code.

We're trying to not change any pulbic APIs before mozilla 1.0, so we'd like to
revisit this after mozilla 1.0 branches.

Comment 21

17 years ago
Comment on attachment 75559 [details] [diff] [review]
Use nsIPresContext to carry focused with flag

This patch is built on a broken source tree. Ignore it.
Attachment #75559 - Attachment is obsolete: true

Comment 22

17 years ago
The script calls element's focus function by asm code. Not from 
eventStateManager. 
Maybe we can modify codes in nsGenericHTMLElement::SetElementFocus(PRBool 
aDoFocus) which called by script and will call content->SetFocus(presContext). 

Updated

17 years ago
QA Contact: madhur → rakeshmishra

Comment 23

16 years ago
aaronl:Is this bug still useful, or we can close it?

Comment 24

16 years ago
-> Reassign to Blizzard as this is part of the focus refactoring in bug 178324.

Note, we don't need this anymore to fix bug 28583.
Assignee: pete.zha → blizzard
Blocks: 83552, 178324

Updated

16 years ago
QA Contact: rakeshmishra → trix

Updated

16 years ago
Target Milestone: mozilla1.0 → ---

Comment 25

15 years ago
Removing as direct dependency of bug 83552 since it is dependent on focus
refactoring bug. We don't need this for anything in particular but it would be
good to do if the focus logic was ever refactored.
No longer blocks: 83552
QA Contact: trix → events
No longer blocks: 178324
Depends on: 178324
nsIFocusManager::GetLastFocusMethod can be used now.
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.