Clicking link in message does not bring Navigator window to the front

VERIFIED FIXED in mozilla0.9.1

Status

P3
major
VERIFIED FIXED
18 years ago
7 years ago

People

(Reporter: tarahim, Assigned: danm.moz)

Tracking

({regression})

Trunk
mozilla0.9.1
regression

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments)

(Reporter)

Description

18 years ago
M18-2000083008

Comment 1

18 years ago
Do you currently have a browser window open?  I think this is mscott's.  
Assignee: putterman → mscott

Comment 2

18 years ago
Confirming in 20000991520 OS9
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

18 years ago
Summary: Clicking a URL in the message does not bring the Navigator window to the front. → Clicking link in message does not bring Navigator window to the front

Updated

18 years ago
QA Contact: lchiang → laurel
(Reporter)

Comment 3

18 years ago
*** Bug 67922 has been marked as a duplicate of this bug. ***
I see this on the trunk mac builds, but not on linux.

probably a bug for danm, and not mscott.

to reproduce this, open browser window and mail window, and have mail window in
front.  view a message with a html link, click on the link.  the page loads in
the browser window, but the browser window isn't brought to front.

Comment 5

18 years ago
this is a dup of an old mac bug. i'll try to find it.

Comment 6

18 years ago
I see this on Linux build.
OS: Mac System 8.6 → All
Hardware: Macintosh → All
still seeing this [2001.03.12.05, linux moz]. nominating.

danm, should this be yours?
Keywords: mozilla0.9, nsbeta1
(Assignee)

Comment 8

18 years ago
What, is this a case where we actually have an opportunity to focus a window, and 
don't? Um sure, it might be my bug more than somebody else's. I hope Scott can 
find a duplicate bug, but I'll take it in the meantime. In doing so, I'm 
unnominating it from mozilla 0.9, since there's no chance I'll get to it by then 
and it's not "feature complete" work, anyway.
Keywords: mozilla0.9
Target Milestone: --- → mozilla1.0
(Assignee)

Updated

18 years ago
Assignee: mscott → danm
(Assignee)

Comment 9

18 years ago
.

Comment 10

18 years ago
WFM with Linux build.
(Reporter)

Comment 11

18 years ago
It is WFM in 2001033004trunk for Mac.
Resolving as such since it has been reported to be WFM in Linux build. 
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → WORKSFORME

Comment 12

18 years ago
Worksforme with apr4 build, mac OS 9.0, win98. Can't test on linux right now,
assume OK from last comments.
Status: RESOLVED → VERIFIED

Comment 13

18 years ago
*** Bug 76704 has been marked as a duplicate of this bug. ***

Comment 14

18 years ago
From bug 76704:

> ------- Additional Comments From laurel@netscape.com 2001-04-19 12:08 -------
>
> It works for me usually first time in a session, but won't work after that.

Status: VERIFIED → REOPENED
Resolution: WORKSFORME → ---
(Reporter)

Comment 15

18 years ago
adding regression KW.
It was working in 0417 build but not in 0419 build.
Keywords: regression

Comment 16

18 years ago
*** Bug 77055 has been marked as a duplicate of this bug. ***

Comment 17

18 years ago
*** Bug 77213 has been marked as a duplicate of this bug. ***

Comment 18

18 years ago
This makes mail pretty hard to use.  It was working until recently on Win32 and
just broke.  I'm changing the severity to major and would like to ask that the
priority be raised and that the target milestone can be brought in (figuring out
what happened and getting it into 0.9 would be even better).
Severity: normal → major
Keywords: mailtrack

Updated

18 years ago
Whiteboard: mozilla 0.9 radar

Comment 19

18 years ago
*** Bug 77223 has been marked as a duplicate of this bug. ***

Updated

18 years ago
Whiteboard: mozilla 0.9 radar

Comment 20

18 years ago
Good God I hate this bug.  Please PLEASE fix this.
Keywords: nsCatFood
(Assignee)

Comment 21

18 years ago
An impressive number of people have dropped by and made personal requests to get 
this bug fixed. Though none of them have come bearing doughnuts, it looks like 
this is my chance to reduce the pain in the world.

The regression was caused in rev 1.267 of nsDocShell.cpp. That change taught 
nsDocShell::SetFocus to set the focus on the next contained frame; before that 
change the method just set the focus to the docshell's window. In this case there 
is no subframe wanting focus, and SetFocus does nothing. This patch fixes the bug 
by reinstating the original "focus the whole docshell" clause in the "no 
subframe" case.

Index: nsDocShell.cpp
===================================================================
RCS file: /cvsroot/mozilla/docshell/base/nsDocShell.cpp,v
retrieving revision 1.277
diff -u -r1.277 nsDocShell.cpp
--- nsDocShell.cpp	2001/04/25 02:04:31	1.277
+++ nsDocShell.cpp	2001/04/27 02:13:32
@@ -2025,6 +2025,14 @@
     nsIFrame* focusFrame = nsnull;
     presShell->GetPrimaryFrameFor(focusContent, &focusFrame);
     esm->ChangeFocus(focusContent, focusFrame, PR_TRUE);
+  } else {
+    nsCOMPtr<nsIScriptGlobalObject> sgo;
+    document->GetScriptGlobalObject(getter_AddRefs(sgo));
+    if (sgo) {
+      nsCOMPtr<nsIDOMWindowInternal> domwin(do_QueryInterface(sgo));
+      if (domwin)
+        domwin->Focus();
+    }
   }
 
   return NS_OK;
Status: REOPENED → ASSIGNED
Whiteboard: patch wants review
Target Milestone: mozilla1.0 → mozilla0.9.1

Comment 22

18 years ago
maybe attinasi wants to r=/sr= this

Comment 23

18 years ago
I'd be happy to! [s]r=attinasi
It's important to test to make sure that this doesn't accidentally cause windows
to raise themselves accidentally.  I get so much hate mail about that issue you
wouldn't believe it.

Also, if you want the window to actually raise itself on linux you have to use
nsIWidget::SetFocus(PR_TRUE) to get the window to raise.  There's a default
PR_FALSE arg on that function call now.
(Assignee)

Comment 25

18 years ago
  I'm calling nsIDOMWindowInternal::Focus, which eventually becomes an 
nsIWidget::SetFocus(PR_TRUE). And after some playing, I can't tell that the patch 
causes windows to come to the front any more often than before, except in the one 
case covered by this bug. Specifically, the patch preventing a text field's 
getting focus from activating the window is unaffected.
  The patch seems safe. Unwilling focus maven hyatt likes it. But I'd still like 
to get an OK from either bryner or saari, who know about these things.
The code itself looks reasonable, no doubt there.  I would say check it in with
but expect that if it does cause problems you will hear about it. :)

Updated

18 years ago
Whiteboard: patch wants review → a=drivers for branch checkin

Comment 27

18 years ago
This works great for me on Linux, thanks dan.  Can we check this in?
Keywords: patch

Comment 28

18 years ago
Created attachment 32537 [details] [diff] [review]
patch version of danm's fix

Comment 29

18 years ago
See, you can't change anything in this system without side effects...

Anyway, the patch looks reasonable, but please test the living daylights out of
it. Someone with the patch applied, come get me and I'll go through a set of
tests with you for the r=saari.
r=bryner
(Assignee)

Comment 31

18 years ago
patch is in, but trunk only (not on the 0.9 branch).
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED

Comment 32

18 years ago
verified on win2k comm build, linux mozilla build.
Status: RESOLVED → VERIFIED
(Assignee)

Comment 33

18 years ago
Reopening. Waterson's fix for bug 82073, checked in this morning, regressed 
this.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 34

18 years ago
I'll attach a more robust version of the same fix. I kind of thought I should 
have done this in the first place, but went with the simpler one because it left 
Blame with a clear record. But that made a less robust fix, and it was just 
broken. This new version falls back to focusing the entire document in many more 
cases, rather than simply bailing.
Status: REOPENED → ASSIGNED
Whiteboard: a=drivers for branch checkin
Target Milestone: mozilla0.9.1 → mozilla0.9.2
(Assignee)

Comment 35

18 years ago
Created attachment 35702 [details] [diff] [review]
"rewrite" of SetFocus; really it's the same but is more likely to hit the fallback
(Assignee)

Updated

18 years ago
Depends on: 82073

Comment 36

18 years ago
I don't have a respin build so I can't see this in action, but I think this is a
0.9.1 stopper. It sounds like you have the fix, so I hope it can get checked in.
r=bryner for danm's last patch
(Assignee)

Comment 38

18 years ago
  'preciate the r= Brian. I wonder if you could do it again. Rod checked in a 
big change to that method yesterday, so the patch is now useless. Attaching 
another one. As before, it's basically just shoving things around; moving 
objects' declaration and creation closer to where they're used and preferring to 
fail over into focusing the whole document, rather than bailing.
  Also retargeting to 0.9.1 at putterman's request.
Target Milestone: mozilla0.9.2 → mozilla0.9.1
(Assignee)

Comment 39

18 years ago
Created attachment 35900 [details] [diff] [review]
new "rewrite," like before, but post rods' recent changes

Updated

18 years ago
Whiteboard: updated patch needs re-review
r=bryner

Comment 41

18 years ago
sr=hyatt
(Assignee)

Comment 43

18 years ago
patch is in; bug goes down once again.
Status: ASSIGNED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED
Whiteboard: updated patch needs re-review
(Reporter)

Comment 44

18 years ago
i have not seen this work in 20010528 and 0529 trunk builds. What happened?

Comment 45

18 years ago
I can verify thta this is fixed for mozilla win32 build 052904 on win2K.

Comment 46

18 years ago
This is working OK for me with may29 commercial build:
2001-05-29-04 win98
2001-05-29-08 linux rh6.2

Not OK on mac OS 9.0 with 2001-05-29-08. Will double-check on a few machines
before reopening for mac.

Comment 47

18 years ago
Reopening for mac...
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 48

18 years ago
The Mac-specific problem was the same reason as bug 82413; another variant of bug 
82368. It's fixed too, now.
Status: REOPENED → RESOLVED
Last Resolved: 18 years ago18 years ago
Resolution: --- → FIXED

Comment 49

18 years ago
OK mac OS 9.0 using may30 commercial trunk build.
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.