300284 - Unable to shift-tab out of Compose body into subject line (plain text or HTML).

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Stephen Donner [:stephend] Not actively reading bugmail]

Build ID: 2005-07-10-05, Windows XP SeaMonkey trunk.

Summary: Unable to shift-tab out of Compose body into subject line (plain text
or HTML).

Steps to Reproduce:

1. Open mail/news, click Compose.
2. Place the cursor in the body window (HTML or plain text doesn't matter).
3. Hold down Shift, press Tab.

Expected Results:

Shift-tab should backwards9cycle focus into the subject line from the compose body.

Actual Results:

No focus move happens.

Comment 1

[Boris Zbarsky [:bzbarsky]]

This is a regression; this used to work sometime in early 2005.  Unfortunately,
Andrew Schultz and I get different dates for the regression range and neither
range has any relevant checkins...

Comment 2

[[e-mail]]

Early 2005? The checkins I'd like to suspect most were aaron's focus changes
around November 2004, so CCing hiim in case he has any ideas. In the mean time
I've traced the code and it does reach ShiftFocusInternal which I expected.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Well, Andrew and I were seeing results differing by a month (he had it not
working as of mid-January, while over here it seemed to work till mid-February)....

Comment 4

[Andrew Schultz]

With valgrind, I get:
Conditional jump or move depends on uninitialised value(s)
 nsEventStateManager::GetNextTabbableContent(nsIContent*, nsIContent*,
nsIFrame*, int, int, nsIContent**, nsIFrame**) (nsEventStateManager.cpp:3480)
 nsEventStateManager::ShiftFocusInternal(int, nsIContent*)
(nsEventStateManager.cpp:3226)
 nsEventStateManager::ShiftFocus(int, nsIContent*) (nsEventStateManager.cpp:3114)
 nsEventStateManager::PostHandleEvent(nsPresContext*, nsEvent*, nsIFrame*,
nsEventStatus*, nsIView*) (nsEventStateManager.cpp:2094)

line 3477 is 

    PRInt32 tabIndex;
    nsIContent* currentContent = (*aResultFrame)->GetContent();
    (*aResultFrame)->IsFocusable(&tabIndex);
    if (tabIndex >= 0) {

some implementations of IsFocusable might not set aTabIndex.  nsXULElement
actually uses the incoming value as a default.

Relevant code comes from bug 250006.

Comment 5

[Aaron Leventhal]

Attachment: Watch out for unintialized out values! Always set *aTabindex if aTabIndex != nsnull

Andrew Schultz, nice find -- thanks.

Comment 6

[Aaron Leventhal]

Checking in nsFrame.cpp;
/cvsroot/mozilla/layout/generic/nsFrame.cpp,v  <--  nsFrame.cpp
new revision: 3.569; previous revision: 3.568
done

Comment 7

[Wayne Mery (:wsmwk)]

(been waiting - I'll be so glad to get this back, so THANKS ...)

possible dups 296468  bug 265582  
can't characterize bug 186701 since I'm not MACy, but perhaps worth checking?

Comment 8

[Stephen Donner [:stephend] Not actively reading bugmail]

*** Bug 296468 has been marked as a duplicate of this bug. ***

Comment 9

[Stephen Donner [:stephend] Not actively reading bugmail]

*** Bug 265582 has been marked as a duplicate of this bug. ***

Comment 10

[Mike Cowperthwaite]

Testing with TB 1.0+0722, Win2K, the problem described in the summary has not 
been solved.

Comment 11

[Aaron Leventhal]

Testing with Thunderbird version 1.0+ (20050722) on WinXP. Problem fixed.
WORKSFORME.

Did you download the 7/22 installer executable from
http://ftp.mozilla.org/pub/mozilla.org/thunderbird/nightly/latest-trunk/
?

Sometimes it takes a while for the new day's installer to be posted to that
link. My suspicion is that you are really running 7/21.

Comment 12

[Stephen Donner [:stephend] Not actively reading bugmail]

works just fine for me with version 1.0+ (20050722) of Thunderbird and build
2005-07-22-05 on Windows XP.

Comment 13

[Andrew Schultz]

If it was uninitialized before and the default is now "not tabbable" then the
new behavior should be not tabbable (but consistently).  So I'm not sure how the
patch could fix the bug.

Comment 14

[Aaron Leventhal]

(In reply to comment #13)
> If it was uninitialized before and the default is now "not tabbable" then the
> new behavior should be not tabbable (but consistently).  So I'm not sure how the
> patch could fix the bug.

Andrew, have you tried a 7/22 build?

It was getting an ancestor frame in the editor to the currently focused one. The
unitialized value was coming out positive, which changed whether it went into
the following if branch:

    PRInt32 tabIndex;
    nsIContent* currentContent = (*aResultFrame)->GetContent();
    (*aResultFrame)->IsFocusable(&tabIndex);
    if (tabIndex >= 0) {
...
      else if ((aIgnoreTabIndex || mCurrentTabIndex == tabIndex) &&
          currentContent != aStartContent) {
        NS_ADDREF(*aResultNode = currentContent);
        return NS_OK;
      }
    }

That code was executed when it was not supposed to be. I haven't gone through
exactly what happened when it is, except to note that fixing the uninitialized
value makes that not happen and fixes the bug in all my tests. Stephen Donner
also noticed it is fixed in his build.

Comment 15

[Asa Dotzler [:asa]]

Fixed with 2005-07-22 builds.

Comment 16

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified using build 2005-07-23-05 on Windows XP SeaMonkey trunk.