564115 - activating the browser window scrolls the document (to show the focused text control)

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[Karl Tomlinson (:karlt)]

STR:
1. Load bug 353539
2. Click in the "Find" control (to focus)
3. Scroll down with the mouse (to look at something else).
4. Deactivate the window (by mousing over another window or whatever)
5. Re-activate the browser window.

Expected results:
Not much

Actual results:
I'm looking at the Find control again.

I guessing (or actually tn guessed) this was caused by 353539.

Comment 1

[(no longer active)]

Attachment: Patch (v1)

Basically I set the event to only trigger when the frame was not previously focused.

Comment 2

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

+  const PRBool doScroll = aOn != isFocusedRightNow;

Shouldn't we only scroll if aOn && !isFocusedRightNow? It looks like you'd be scrolling into view on blur... If so, you probably should add a test for that.

Comment 3

[(no longer active)]

Attachment: Patch (v1.1)

(In reply to comment #2)
> +  const PRBool doScroll = aOn != isFocusedRightNow;
> 
> Shouldn't we only scroll if aOn && !isFocusedRightNow? It looks like you'd be
> scrolling into view on blur... If so, you probably should add a test for that.

Well, actually this condition was a leftover from a previous try, which I forgot to tweak.  By the time that this code is executed, we know that aOn == PR_TRUE because otherwise the function would've returned.  So I guess the condition can be simplified to !isFocusedRightNow.

Comment 4

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

Comment on attachment 443981 [details] [diff] [review]
Patch (v1.1)

+  // tell the caret to use our selection
+  if (!isFocusedRightNow) {
+    caret->SetCaretDOMSelection(ourSel);
+  }

Make this unconditional since it's a no-op if isFocusedRightNow.

Also I would get rid of the doScroll variable, the code would be clear without it.

Comment 5

[(no longer active)]

Attachment: Patch for landing

With comments addressed.

Comment 6

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/6a779c75c7bf