Last Comment Bug 873449 - No title or icon in titlebar for popup windows
: No title or icon in titlebar for popup windows
Status: RESOLVED FIXED
[Australis:M5]
: regression
Product: Firefox
Classification: Client Software
Component: General (show other bugs)
: Trunk
: x86 Windows 7
-- normal (vote)
: Firefox 28
Assigned To: :Gijs
:
:
Mentors:
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:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
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
STR:

1. Open a window using: window.open("http://www.mozilla.org/", "_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]
Patch

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]
Patch

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.