If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

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

RESOLVED FIXED in mozilla1.9.3a5

Status

()

Core
Layout: Form Controls
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: karlt, Assigned: Ehsan)

Tracking

({regression})

Trunk
mozilla1.9.3a5
x86
Linux
regression
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

8 years ago
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.
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Keywords: regression
Created attachment 443965 [details] [diff] [review]
Patch (v1)

Basically I set the event to only trigger when the frame was not previously focused.
Attachment #443965 - Flags: review?(roc)
+  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.
Created attachment 443981 [details] [diff] [review]
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.
Attachment #443965 - Attachment is obsolete: true
Attachment #443981 - Flags: review?(roc)
Attachment #443965 - Flags: review?(roc)
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.
Attachment #443981 - Flags: review?(roc) → review+
Created attachment 443988 [details] [diff] [review]
Patch for landing

With comments addressed.
http://hg.mozilla.org/mozilla-central/rev/6a779c75c7bf
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
You need to log in before you can comment on or make changes to this bug.