Last Comment Bug 873449 - No title or icon in titlebar for popup windows
: No title or icon in titlebar for popup windows
: regression
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: x86 Windows 7
-- normal (vote)
: Firefox 28
Assigned To: :Gijs
Depends on:
Blocks: australis-tabs-win 813802
  Show dependency treegraph
Reported: 2013-05-17 06:03 PDT by :Gijs
Modified: 2014-02-06 05:08 PST (History)
3 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch (1.34 KB, patch)
2013-05-17 06:36 PDT, :Gijs
dao+bmo: review-
Details | Diff | Splinter Review
Patch v2 (1.34 KB, patch)
2013-05-17 08:58 PDT, :Gijs
dao+bmo: review+
Details | Diff | Splinter Review

Description User image :Gijs 2013-05-17 06:03:17 PDT

1. Open a window using:"", "_new", "height=500&width=500")
2. Try to read the page title from the top of the window

AR: no page title nor icon
ER: page title and icon, as on Nightly

AFAICT this is because in updateTitlebarDisplay, we're now only using gInPrintPreviewMode to determine whether to show the titlebar and change the chrome margins.

I believe this also causes some of our test failures, eg. the queryCaretRectWin failures in mochitest-other (chrome).
Comment 1 User image :Gijs 2013-05-17 06:36:20 PDT
Created attachment 751012 [details] [diff] [review]

I believe the original removal of the window.menubar.visible bits was a mistake (as these are barprops and don't necessarily say anything about whether the actual XUL element is visible), so I've more or less changed that back (although calling it displayAppButton didn't make sense so I've called it drawInTitlebar instead). I *have* left out the check whether the menubar is autohidden, which I guess won't make sense anymore.

This fixes the issue I was seeing.

It also fixes the test I was originally looking at (test_queryCaretRect), as a side-effect.
Comment 2 User image Dão Gottwald [:dao] 2013-05-17 07:11:23 PDT
Comment on attachment 751012 [details] [diff] [review]

Since this isn't about the app menu button anymore, it's unclear to me why this behavior should depend on window.menubar.visible.

I believe you should use window.toolbar.visible. That's what tabbrowser uses to hide the tab bar in popups.
Comment 3 User image :Gijs 2013-05-17 08:58:02 PDT
Created attachment 751057 [details] [diff] [review]
Patch v2

OK. This works just as well in a quick test, so here we go...
Comment 6 User image Ioana (away) 2014-02-06 05:08:14 PST
Not reproducible locally (05/17 Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20130517 Firefox/24.0).

Note You need to log in before you can comment on or make changes to this bug.