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

RESOLVED FIXED in mozilla1.9.3a5

Status

()

defect
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: karlt, Assigned: Ehsan)

Tracking

({regression})

Trunk
mozilla1.9.3a5
x86
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

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

Updated

9 years ago
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Keywords: regression
Assignee

Comment 1

9 years ago
Posted patch Patch (v1) (obsolete) — Splinter Review
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.
Assignee

Comment 3

9 years ago
Posted patch Patch (v1.1)Splinter Review
(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+
Assignee

Comment 5

9 years ago
With comments addressed.
Assignee

Comment 6

9 years ago
http://hg.mozilla.org/mozilla-central/rev/6a779c75c7bf
Status: ASSIGNED → RESOLVED
Closed: 9 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.