Closed Bug 956286 Opened 11 years ago Closed 11 years ago

nightly started from taskbar icon doesn't get startup focus

Categories

(Firefox for Metro Graveyard :: Shell, defect, P2)

29 Branch
x86_64
Windows 8.1
defect

Tracking

(firefox28 verified, firefox29 verified)

VERIFIED FIXED
Firefox 29
Tracking Status
firefox28 --- verified
firefox29 --- verified

People

(Reporter: zao, Assigned: jimm)

References

Details

(Whiteboard: [beta28] [defect] p=2)

Attachments

(1 file, 2 obsolete files)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:29.0) Gecko/20100101 Firefox/29.0 (Beta/Release)
Build ID: 20140102030203

Steps to reproduce:

Install Nightly (2014-01-02) on Windows 8.1.
Click the icon pinned to taskbar to launch the Desktop flavor of Nightly.


Actual results:

The Nightly window that appears does not have window focus, this includes the Safe Mode box if you force it with the Shift key.

The bug only occurs when CommandExecuteHandler.exe is used as the process parent and launching from the taskbar. Running firefox.exe directly doesn't demonstrate the bug, nor does running Nightly from the Start Screen.


Expected results:

The Nightly window that appears should gain focus the same way that any other application gains it when they start.
Component: Untriaged → Install/Update
OS: Linux → Windows 8.1
Product: Firefox → Firefox for Metro
I presume this isn't actually about Metro but it's caused by recent Metro changes.
Status: UNCONFIRMED → NEW
Ever confirmed: true
I was only able to reproduce this once for the first launch after a switch. After that launch the browser had foreground status. Can you confirm the same behavior?
(In reply to Jim Mathies [:jimm] from comment #2)
> I was only able to reproduce this once for the first launch after a switch.
> After that launch the browser had foreground status. Can you confirm the
> same behavior?

Oh, nm, having another window on the desktop reproduces every launch.
Whiteboard: [triage]
Whiteboard: [triage] → [beta28] [defect] p=0
Assignee: nobody → jmathies
Blocks: metrov1it22
No longer blocks: metrov1backlog
Status: NEW → ASSIGNED
Priority: -- → P2
QA Contact: jbecerra
Whiteboard: [beta28] [defect] p=0 → [beta28] [defect] p=2
Reproducible on the 01/07 Firefox 28.0a2.

The window is not in focus when launching Firefox from the taskbar icon, but it is in focus when launching it from the Metro interface.
In some ways this is desired behavior, for example if you tap the find bar edit to bring up the osk, then hit the windows button twice to flip to the start screen and back, the osk will slide back up when fx opens (with the focus set to the find bar edit).

So I think the fix here is to hide the find bar when a flyout is displayed. We do this for the tab strip and the nav bar already.
Jim, I don't entirely understand what you're saying. This bug is about Desktop (not Metro) Nightly not getting focus on being started, which can never be desired behavior? It's filed in Metro because it's most likely caused by Metro changes.
(In reply to Gian-Carlo Pascutto (:gcp) from comment #6)
> Jim, I don't entirely understand what you're saying. This bug is about
> Desktop (not Metro) Nightly not getting focus on being started, which can
> never be desired behavior? It's filed in Metro because it's most likely
> caused by Metro changes.

sorry that comment was supposed to go in bug 952989.
Component: Install/Update → Shell
Attached patch patch v.1 (obsolete) — Splinter Review
Attached patch patch v.1 (obsolete) — Splinter Review
Attachment #8359388 - Attachment is obsolete: true
Attached patch patch v.1Splinter Review
The fix plus some cleanup. The issue here was that the ceh had foreground rights, which we need to hand off to the browser we launch. Switched to CreateProcess so I could specify ShowWindow params and have quick access to the process id.

Note I first tried this with shell execute which can also return the process handle, but it didn't work. Not sure why but CreateProcess did so I'm using that.
Attachment #8359393 - Flags: review?(netzen)
Attachment #8359389 - Attachment is obsolete: true
Comment on attachment 8359393 [details] [diff] [review]
patch v.1

Review of attachment 8359393 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/metro/shell/commandexecutehandler/CommandExecuteHandler.cpp
@@ +605,5 @@
> +  startInfo.cb = sizeof(STARTUPINFO);
> +  startInfo.dwFlags = STARTF_USESHOWWINDOW;
> +  startInfo.wShowWindow = SW_SHOWNORMAL;
> +
> +  BOOL result = 

nit: trailing whitespace

@@ +606,5 @@
> +  startInfo.dwFlags = STARTF_USESHOWWINDOW;
> +  startInfo.wShowWindow = SW_SHOWNORMAL;
> +
> +  BOOL result = 
> +    CreateProcessW(aBrowserPath, const_cast<LPWSTR>(params.GetBuffer()),

nit: Looks like you're trying to use const_cast to remove constness from params.GetBuffer() but I think that's already a non const pointer.

@@ +615,5 @@
> +  }
> +  // Hand off foreground/focus rights to the browser we create. If we don't
> +  // do this the ceh will keep ownership causing desktop firefox to launch
> +  // deactivated.
> +  AllowSetForegroundWindow(procInfo.dwProcessId);

Let's log a warning when this fails
Attachment #8359393 - Flags: review?(netzen) → review+
(In reply to Brian R. Bondy [:bbondy] from comment #11)
> @@ +606,5 @@
> > +  startInfo.dwFlags = STARTF_USESHOWWINDOW;
> > +  startInfo.wShowWindow = SW_SHOWNORMAL;
> > +
> > +  BOOL result = 
> > +    CreateProcessW(aBrowserPath, const_cast<LPWSTR>(params.GetBuffer()),
> 
> nit: Looks like you're trying to use const_cast to remove constness from
> params.GetBuffer() but I think that's already a non const pointer.

that should have been static_cast. updated.
https://hg.mozilla.org/mozilla-central/rev/2fb50e84768b
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
For iteration #22, verified as fixed with latest Nightly on 8.1 64-bit: after clicking the icon pinned to taskbar to launch Nightly in Desktop mode, the Nightly window that appears has focus.
Status: RESOLVED → VERIFIED
Kamil, please verify this is fixed in the latest Aurora build.
Flags: needinfo?(kamiljoz)
Went through the following verification without any issues. Used the following build:
- http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/2014-02-05-00-40-01-mozilla-aurora/

- Ensured that Firefox has focus when launching from the pinned icon on the task bar
- Ensured that Firefox has focus when launching from the desktop shortcut
- Ensured that Firefox has focus when launching from the firefox.exe located in the installation folder
- Ensured that Firefox has focus when switching from metrofx to firefox desktop
- Went through all of the above test cases while I had several applications opened on the desktop
- Went through all of the above test cases using different variations of snapped view
Flags: needinfo?(kamiljoz)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: