1403432 - windowed mode doesn't display current page's title/url as it used to

Status: VERIFIED FIXED

(DevTools :: General, defect, P2)

Description

[Kaiido]

Attachment: screenshot to show the issue

(Not sure if it really needs a bug report but since you plan to land 57 quite soon...)

When inspector is in windowed mode, the top bar used to show the inspected page's title/url.

In 57 (both Beta channel and Developper Edition) it doesn't anymore.
Seems to be already fixed in 58 (Nightly).

Comment 1

[Patrick Brosset <:pbro>]

Thanks for filing. I was able to reproduce. This is indeed a regression. I'll try to look for a regression range.

Comment 2

[Patrick Brosset <:pbro>]

Here's what I found so far:

Last good revision: 4193c11e97aeab5a258d5499bf4a4d9177b27380
First bad revision: 938750d859b85600b84013bd6dea63db1c32d135
Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=4193c11e97aeab5a258d5499bf4a4d9177b27380&tochange=938750d859b85600b84013bd6dea63db1c32d135

So this could have been caused by bug 1401297 or bug 1398582.

@Stephen: Hey, do you know if either of these bugs could be causing this?

Comment 3

[Stephen A Pohl [:spohl]]

Attachment: Patch

We always tracked whether or not we wanted to show the window title internally via nsCocoaWindow's mDrawTitle and it was actually set to false, but before the 10.13 regression fixes, we never used to call NSWindow's setTitleVisibility:. This means that as long as we would not cover the native titlebar by drawing into the titlebar, the window title was visible even though mDrawTitle was false. As part of the visual regression fixes on macOS 10.13, we started calling NSWindow's setTitleVisibility: correctly. Although mstange's floating titlebar fix eventually made that patch unnecessary, it's still the right thing to do and the inspector window should simply set the "drawtitle" attribute to "true". This allows us to track the state correctly via mDrawTitle and we display the window titles again.

Comment 4

[Markus Stange [:mstange]]

The inspector is not a drawsContentsIntoWindowFrame window (it doesn't set the chromemargin attribute). Wouldn't it be better if Widget just always treated windows with separate titlebars as windows that want their title to be displayed? Otherwise we'll have to fix a lot more windows, e.g. the bookmarks manager, the page info window, ...

Comment 5

[Stephen A Pohl [:spohl]]

Comment on attachment 8916297 [details] [diff] [review]
Patch

Yes, I've been thinking about this solution too and meant to ask you about it only to forget about it when I posted my patch. Will address. Thanks!

Comment 6

[Stephen A Pohl [:spohl]]

Attachment: Patch

As suggested. The one unfortunate thing now is that we ignore the choice made in nsXULWindow::SyncAttributesToWidget about whether or not we want to draw window titles. So if we don't draw into the window frame, we can't disable the window titles. This is how things behaved previously on macOS, but it's worth pointing out.

Comment 7

[Stephen A Pohl [:spohl]]

Attachment: Patch

s/true/YES/ for BOOL argument.

Comment 8

[Stephen A Pohl [:spohl]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8bd41749e385016da8d8e333a2796ecba0e22c73
Bug 1403432: Show window titles when the inspector is launched in windowed mode. r=mstange

Comment 9

[Patrick Brosset <:pbro>]

\o/ Thanks Stephen and Markus for this fix!
Do you think this could be uplifted to 57 (where the problem exists)? If so, can you please open the request?

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/8bd41749e385

Comment 11

[Stephen A Pohl [:spohl]]

Comment on attachment 8916302 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/Bug causing the regression]: Regression from macOS 10.13 compatibility fixes (tracked in meta bug 1391790).
[User impact if declined]: Windows with native titlebars (such as the inspector window) will fail to display window titles.
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Yes.
1. Open Inspector (Option-Cmd-C)
2. Click on the icon to "Show in separate window" at the top right of the Inspector panel.
3. Verify that the Inspector window shows the page title in the titlebar.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: This is a very small patch that simply restores the known good behavior from before, i.e. Firefox 56 and below.
[String changes made/needed]: None.

Comment 12

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8916302 [details] [diff] [review]
Patch

New problem with OSX 10.13 compatibility, Beta57+

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/b0466dd6b31a

Comment 14

[Stephen A Pohl [:spohl]]

Fyi: This bug affects macOS version 10.10 or above and is not specific to 10.13.

Comment 15

[Oana Botisan, Desktop QA]

I managed to reproduce the bug using beta 57.0b3 on Mac OS X 10.11 and macOS 10.12.
I retested everything using beta 57.0b8 and latest Nightly 58 on the same platforms, but the bug is not reproducing. Windowed mode now displays a title/url.