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)
Tracking
(firefox28 verified, firefox29 verified)
VERIFIED
FIXED
Firefox 29
People
(Reporter: zao, Assigned: jimm)
References
Details
(Whiteboard: [beta28] [defect] p=2)
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
bbondy
:
review+
|
Details | Diff | Splinter Review |
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.
Updated•11 years ago
|
Component: Untriaged → Install/Update
OS: Linux → Windows 8.1
Product: Firefox → Firefox for Metro
Comment 1•11 years ago
|
||
I presume this isn't actually about Metro but it's caused by recent Metro changes.
Assignee | ||
Updated•11 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee | ||
Comment 2•11 years ago
|
||
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?
Assignee | ||
Comment 3•11 years ago
|
||
(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.
Assignee | ||
Updated•11 years ago
|
Whiteboard: [triage]
Updated•11 years ago
|
Blocks: metrov1backlog
Updated•11 years ago
|
status-firefox28:
--- → ?
status-firefox29:
--- → affected
Keywords: regression,
regressionwindow-wanted
Updated•11 years ago
|
Whiteboard: [triage] → [beta28] [defect] p=0
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → jmathies
Updated•11 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P2
QA Contact: jbecerra
Whiteboard: [beta28] [defect] p=0 → [beta28] [defect] p=2
Comment 4•11 years ago
|
||
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.
Assignee | ||
Comment 5•11 years ago
|
||
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.
Comment 6•11 years ago
|
||
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.
Assignee | ||
Comment 7•11 years ago
|
||
(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.
Assignee | ||
Updated•11 years ago
|
Component: Install/Update → Shell
Assignee | ||
Comment 8•11 years ago
|
||
Assignee | ||
Comment 9•11 years ago
|
||
Attachment #8359388 -
Attachment is obsolete: true
Assignee | ||
Comment 10•11 years ago
|
||
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)
Assignee | ||
Updated•11 years ago
|
Attachment #8359389 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Keywords: regression,
regressionwindow-wanted
Comment 11•11 years ago
|
||
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+
Assignee | ||
Comment 12•11 years ago
|
||
(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.
Assignee | ||
Comment 13•11 years ago
|
||
Comment 14•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
Comment 15•11 years ago
|
||
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
Comment 16•11 years ago
|
||
Comment 17•11 years ago
|
||
Kamil, please verify this is fixed in the latest Aurora build.
Flags: needinfo?(kamiljoz)
Comment 18•11 years ago
|
||
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.
Description
•