Implement the firefox button on xp, classic, and aero basic

RESOLVED FIXED in Firefox 4.0b5

Status

()

Firefox
Theme
RESOLVED FIXED
7 years ago
3 years ago

People

(Reporter: jimm, Assigned: jimm)

Tracking

Trunk
Firefox 4.0b5
x86
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking2.0 beta5+)

Details

Attachments

(13 attachments, 15 obsolete attachments)

41.50 KB, image/png
Details
230.19 KB, image/png
Details
935.57 KB, image/png
Details
231.82 KB, image/png
Details
72.37 KB, image/png
Details
10.10 KB, patch
dao
: review+
Details | Diff | Splinter Review
25.50 KB, image/jpeg
Details
40.99 KB, image/png
Details
20.63 KB, image/png
Details
23.86 KB, image/png
Details
8.84 KB, image/jpeg
Details
14.24 KB, patch
Details | Diff | Splinter Review
7.42 KB, patch
roc
: review+
dao
: review+
Details | Diff | Splinter Review
(Assignee)

Description

7 years ago
This is waiting on bug 574454. Once we have the ability to implement window frame, titlebar, and caption buttons via xul/css, we'll need to add support for this in firefox. This will be for non-glass desktops only, as glass desktops make use of the system frame.
(Assignee)

Updated

7 years ago
Duplicate of this bug: 575854
(Assignee)

Updated

7 years ago
Blocks: 576960
(Assignee)

Updated

7 years ago
blocking2.0: --- → ?
(Assignee)

Comment 2

7 years ago
Moving these down since they will depend on the dimensions of the frame we implement.
Blocks: 578616, 581023
blocking2.0: ? → betaN+
(Assignee)

Updated

7 years ago
Blocks: 582819
(Assignee)

Comment 3

7 years ago
Created attachment 461447 [details] [diff] [review]
patch v.1

first rev., tested on win7. I still need to do testing on xp tomorrow. This also address the dependent bugs related to top window padding.
Assignee: nobody → jmathies
(Assignee)

Comment 4

7 years ago
Note, the diff for browser.xul looks a little scary, but the actual changes involve new header and footer elements for appmenu-button-container. The rest is all indentation.
(Assignee)

Comment 5

7 years ago
Created attachment 461453 [details] [diff] [review]
patch v.1

same patch minus the browser.xul indentation changes.
Attachment #461447 - Attachment is obsolete: true
(Assignee)

Comment 6

7 years ago
morphing.
Summary: Implement custom drawn window frame and titlebar for themed and classic desktops → Implement the firefox button on xp, 2k, and aero basic
(Assignee)

Updated

7 years ago
Summary: Implement the firefox button on xp, 2k, and aero basic → Implement the firefox button on xp, classic, and aero basic
(Assignee)

Comment 7

7 years ago
Created attachment 461732 [details] [diff] [review]
patch v.2
Attachment #461453 - Attachment is obsolete: true
(Assignee)

Comment 8

7 years ago
Created attachment 461733 [details] [diff] [review]
patch v.2
Attachment #461732 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Blocks: 576312
(Assignee)

Updated

7 years ago
Blocks: 582020
(Assignee)

Updated

7 years ago
Blocks: 582632
(Assignee)

Updated

7 years ago
Blocks: 578729
(Assignee)

Comment 9

7 years ago
Comment on attachment 461733 [details] [diff] [review]
patch v.2

Dao, one open question I have on this is how to best handle RTL operating systems. I added :

#titlebar:-moz-locale-dir(rtl) {
  -moz-transform: scaleX(-1);
}

But I don't think that's the right way to do this. The elements in the titlebar had padding assigned to them via moz css styles (bug 574454) which switch out depending on the rtl state of the os. So just flipping the titlebar isn't right. Curious if you could offer some feedback on how that might be done along with a review of the other changes.
Attachment #461733 - Flags: review?(dao)
(Assignee)

Comment 10

7 years ago
Found this screen cap of a rtl window on the web:

http://www.microsoft.com/msj/0499/multilangunicode/multilangfig05.gif

Maybe the mirroring is correct, and I should not be flipping button padding in the patch in bug 574454? Looks like approach would work as well.
Comment on attachment 461733 [details] [diff] [review]
patch v.2

use WINSTRIPE_AERO or browser-aero.css instead of [os=foo]

I don't think you need to do anything for rtl, it should work automatically.

Shouldn't the windowdragbox binding be applied on #titlebar rather than #titlebar-spacer?
Attachment #461733 - Flags: review?(dao) → review-
(Assignee)

Comment 12

7 years ago
(In reply to comment #11)
> Comment on attachment 461733 [details] [diff] [review]
> patch v.2
> 
> use WINSTRIPE_AERO or browser-aero.css instead of [os=foo]
> 

updated.

> I don't think you need to do anything for rtl, it should work automatically.

Ok, I'll remove the rtl css and update the patch in bug 574454.

> 
> Shouldn't the windowdragbox binding be applied on #titlebar rather than
> #titlebar-spacer?

Yes, that was misplaced. Updated.
(Assignee)

Comment 13

7 years ago
(In reply to comment #12)
> > 
> > Shouldn't the windowdragbox binding be applied on #titlebar rather than
> > #titlebar-spacer?
> 
> Yes, that was misplaced. Updated.

Hmm, actually, if I place that up in #titlebar, it robs the caption buttons of mouse events. I'll try to track down the cause.
(Assignee)

Comment 14

7 years ago
Created attachment 462066 [details] [diff] [review]
patch v.3
Attachment #461733 - Attachment is obsolete: true
(Assignee)

Comment 15

7 years ago
Created attachment 462067 [details] [diff] [review]
patch v.3

Ok, this addresses all the comments. The caption buttons are now xul buttons so they get mouse input.
Attachment #462066 - Attachment is obsolete: true
Attachment #462067 - Flags: review?(dao)
Comment on attachment 462067 [details] [diff] [review]
patch v.3

>+    document.getElementById("titlebar").hidden = false;
>+    document.getElementById("titlebar-buttonbox").hidden = false;
>+  } else {
>     document.documentElement.removeAttribute("chromemargin");
>+    document.getElementById("titlebar").hidden = true;
>+    document.getElementById("titlebar-buttonbox").hidden = true;
>+  }

Why does titlebar-buttonbox get hidden separately when titlebar is already hidden?

>+function updateTitlebarButtonDisplay() {
>+#ifdef XP_WIN
>+  /* must be updated after the window is visible for vista and up */

Why?
(Assignee)

Comment 17

7 years ago
(In reply to comment #16)
> Comment on attachment 462067 [details] [diff] [review]
> patch v.3
> 
> >+    document.getElementById("titlebar").hidden = false;
> >+    document.getElementById("titlebar-buttonbox").hidden = false;
> >+  } else {
> >     document.documentElement.removeAttribute("chromemargin");
> >+    document.getElementById("titlebar").hidden = true;
> >+    document.getElementById("titlebar-buttonbox").hidden = true;
> >+  }
> 
> Why does titlebar-buttonbox get hidden separately when titlebar is already
> hidden?
> 
> >+function updateTitlebarButtonDisplay() {
> >+#ifdef XP_WIN
> >+  /* must be updated after the window is visible for vista and up */
> 
> Why?

This is due to an issue with windows in retrieving information regarding the proper placement & size of caption buttons. On vista and up, the only way to get this info is to query a window with these buttons via a windows message. To get accurate data the window has to be visible when you query it.

Layout queries the theme code for minimum widget size and padding values before the main window is displayed, unless the element in question is collapsed. So I kept these buttons in that state until delayedStartup executes and the main window is visible which insures the data is available.

(I tried doing this in a pageshow handler for main-window as well, but pageshow didn't fire.) 

Another possible fix here might be to trigger a reflow or somehow tell layout to requery for these values once the data is available, maybe through a theme changed event sent from widget. That would probably cause a perf regression though. So I went with the collapsed trick instead.
delayedStartup doesn't have much to do with window visibility, it's just an arbitrary point after the load event (which I think would fire after pageshow if pageshow was fired at all).
(Assignee)

Comment 19

7 years ago
(In reply to comment #18)
> delayedStartup doesn't have much to do with window visibility, it's just an
> arbitrary point after the load event (which I think would fire after pageshow
> if pageshow was fired at all).

Alright, rather than that, the 'MozAfterPaint' seems to work since the first paint on the window doesn't arrive until after the initial show. Will post a new patch.
(Assignee)

Comment 20

7 years ago
Created attachment 462391 [details] [diff] [review]
patch v.4
Attachment #462067 - Attachment is obsolete: true
Attachment #462067 - Flags: review?(dao)
Comment on attachment 462391 [details] [diff] [review]
patch v.4

>+#ifdef MENUBAR_CAN_AUTOHIDE
>+  window.addEventListener("MozAfterPaint", updateTitlebarButtonDisplay, false);
>+#endif
...
>+function updateTitlebarButtonDisplay() {
>+  window.removeEventListener("MozAfterPaint", updateTitlebarButtonDisplay, false);
>+  /* must be updated after the window is visible for vista and up */
>+  document.getElementById("titlebar-buttonbox").style.visibility = "visible";
>+}

Doesn't look like updateTitlebarButtonDisplay needs to be global. Can you declare it inline?

>-  if (displayAppButton)
>+  if (displayAppButton) {
>     document.documentElement.setAttribute("chromemargin", "0,-1,-1,-1");
>-  else
>+    document.getElementById("titlebar").hidden = false;
>+    document.getElementById("titlebar-buttonbox").hidden = false;
>+  } else {
>     document.documentElement.removeAttribute("chromemargin");
>+    document.getElementById("titlebar").hidden = true;
>+    document.getElementById("titlebar-buttonbox").hidden = true;
>+  }

Same question as before, why is #titlebar-buttonbox explicitly hidden/shown when #titlebar already has the same state?

>--- a/browser/themes/winstripe/browser/browser-aero.css
>+++ b/browser/themes/winstripe/browser/browser-aero.css

>+/* aesthetic - push the fx button off the top window border */
>+#main-window[sizemode="normal"] > #titlebar > #titlebar-content > #appmenu-button-container {
>+  margin-top: 2px;
>+}

Is this dependent on the OS theme?

>+#titlebar {
>+  -moz-appearance:-moz-window-titlebar;
>+  /* we only need to the middle section, hide the edges of the
>+  theme background beyond the window frame. */
>+  margin-left: -15px;
>+  margin-right: -15px;
>+  visibility: visible;
>+  -moz-binding: url("chrome://global/content/bindings/general.xml#windowdragbox");
>+}

visibility: visible; looks redundant here

>+#titlebar-buttonbox {
>+  -moz-appearance: -moz-window-button-box;
>+  -moz-box-align: start;
>+  /* will be updated after the window is visible on vista and up */
>+  visibility: collapse;
>+}

Can the visibility be handled completely in app code? Can you just use the 'collapse' attribute?

>+#titlebar[os="xp"] > #titlebar-content > titlebar-buttonbox {
>+  visibility: visible;
>+}

This is obsolete.

>+#titlebar-close {
>+  -moz-appearance: -moz-window-button-close;
>+  margin-left: 1px;
>+  margin-right: 0px;
>+}
...
>+/* windows classic titlebar treatment */
...
>+  #titlebar-min {
>+    margin-right: 0px !important;
>+  }
>+
>+  #titlebar-max {
>+    margin-left: 0px !important;
>+  }
>+}

-moz-margin-end / -moz-margin-start?
(Assignee)

Comment 22

7 years ago
(In reply to comment #21)
> Comment on attachment 462391 [details] [diff] [review]
> patch v.4
> 
> >+#ifdef MENUBAR_CAN_AUTOHIDE
> >+  window.addEventListener("MozAfterPaint", updateTitlebarButtonDisplay, false);
> >+#endif
> ...
> >+function updateTitlebarButtonDisplay() {
> >+  window.removeEventListener("MozAfterPaint", updateTitlebarButtonDisplay, false);
> >+  /* must be updated after the window is visible for vista and up */
> >+  document.getElementById("titlebar-buttonbox").style.visibility = "visible";
> >+}
> 
> Doesn't look like updateTitlebarButtonDisplay needs to be global. Can you
> declare it inline?

Updated.

> >+    document.getElementById("titlebar-buttonbox").hidden = false;
> 
> Same question as before, why is #titlebar-buttonbox explicitly hidden/shown
> when #titlebar already has the same state?

Sorry, didn't update this. That's left over code from testing buttons display, looks like it can come out.

> 
> >--- a/browser/themes/winstripe/browser/browser-aero.css
> >+++ b/browser/themes/winstripe/browser/browser-aero.css
> 
> >+/* aesthetic - push the fx button off the top window border */
> >+#main-window[sizemode="normal"] > #titlebar > #titlebar-content > #appmenu-button-container {
> >+  margin-top: 2px;
> >+}
> 
> Is this dependent on the OS theme?

More the os really, it has to do with the way the fx button hangs on a particular window frame. So with glass/aero basic, IMHO 2px looks "right", with xp's themes, 1 px looks "right", and with classic, 0px looks right. If you want I can take this out and you can tweak these settings yourself. As the comment states, it's purely aesthetic and based on my own perception of how things should look. (I believe the 2px offset though was already there for aero desktops.)

> 
> >+#titlebar {
> >+  -moz-appearance:-moz-window-titlebar;
> >+  /* we only need to the middle section, hide the edges of the
> >+  theme background beyond the window frame. */
> >+  margin-left: -15px;
> >+  margin-right: -15px;
> >+  visibility: visible;
> >+  -moz-binding: url("chrome://global/content/bindings/general.xml#windowdragbox");
> >+}
> 
> visibility: visible; looks redundant here

removed.

> 
> >+#titlebar-buttonbox {
> >+  -moz-appearance: -moz-window-button-box;
> >+  -moz-box-align: start;
> >+  /* will be updated after the window is visible on vista and up */
> >+  visibility: collapse;
> >+}
> 
> Can the visibility be handled completely in app code? Can you just use the
> 'collapse' attribute?

will try that and see.

> 
> >+#titlebar[os="xp"] > #titlebar-content > titlebar-buttonbox {
> >+  visibility: visible;
> >+}
> 
> This is obsolete.

nixed.

> 
> >+#titlebar-close {
> >+  -moz-appearance: -moz-window-button-close;
> >+  margin-left: 1px;
> >+  margin-right: 0px;
> >+}
> ...
> >+/* windows classic titlebar treatment */
> ...
> >+  #titlebar-min {
> >+    margin-right: 0px !important;
> >+  }
> >+
> >+  #titlebar-max {
> >+    margin-left: 0px !important;
> >+  }
> >+}
> 
> -moz-margin-end / -moz-margin-start?

That's what I was looking for to solve the rtl problem! thanks. Will update and test.
(Assignee)

Comment 23

7 years ago
Created attachment 462434 [details] [diff] [review]
patch v.5
Attachment #462391 - Attachment is obsolete: true
Comment on attachment 462434 [details] [diff] [review]
patch v.5

Mostly nits:

>+#ifdef MENUBAR_CAN_AUTOHIDE
>+  // update the visibility of the titlebar buttons after the window is
>+  // displayed. (required by theme code.)
>+  window.addEventListener("MozAfterPaint", function () {
>+      window.removeEventListener("MozAfterPaint", arguments.callee, false);
>+      document.getElementById("titlebar-buttonbox").removeAttribute("collapsed");
>+    }, false);

s/removeAttribute("collapsed")/collapsed = false/

reduce the indentation of the last three lines by 2 spaces each

>   document.getElementById("appmenu-button-container").hidden = !displayAppButton;
> 
>-  if (displayAppButton)
>+  if (displayAppButton) {
>     document.documentElement.setAttribute("chromemargin", "0,-1,-1,-1");
>-  else
>+    document.getElementById("titlebar").hidden = false;
>+  } else {
>     document.documentElement.removeAttribute("chromemargin");
>+    document.getElementById("titlebar").hidden = true;
>+  }

document.getElementById("appmenu-button-container").hidden also seems redundant now.

By the way, can we get rid of appmenu-button-container in favor of titlebar-content?

>--- a/browser/themes/winstripe/browser/browser-aero.css
>+++ b/browser/themes/winstripe/browser/browser-aero.css

>+/* aesthetic - push the fx button off the top window border */
>+#main-window[sizemode="normal"] > #titlebar > #titlebar-content > #appmenu-button-container {
>+  margin-top: 2px;
>+}

>--- a/browser/themes/winstripe/browser/browser.css
>+++ b/browser/themes/winstripe/browser/browser.css

>+/* aesthetic - push the fx button off the top window border */
>+%ifndef WINSTRIPE_AERO
>+#main-window[sizemode="normal"] > #titlebar > #titlebar-content > #appmenu-button-container {
>+  margin-top: 1px;
> }
> %endif

You could actually avoid browser-aero.css here and do this:

foo {
%ifdef WINSTRIPE_AERO
  margin-top: 2px;
%else
  margin-top: 1px;
%endif
}

>+@media all and (-moz-windows-classic) {
>+  #main-window[sizemode="normal"] > #titlebar > #titlebar-content > #appmenu-button-container {
>+    margin-top: 0px !important;
>+  }
>+
>+  #titlebar-close {
>+    min-width: 0px !important;
>+  }
>+
>+  #titlebar-min {
>+    -moz-margin-end: 0px !important;
>+    min-width: 0px !important;
>+  }
>+
>+  #titlebar-max {
>+    -moz-margin-start: 0px !important;
>+    min-width: 0px !important;
>+  }
>+}

s/0px/0/

Isn't the default button min-width always wrong, not just for classic?

Does your patch put the window controls in the tab order?
(Assignee)

Comment 25

7 years ago
(In reply to comment #24)
> Comment on attachment 462434 [details] [diff] [review]
> 
> By the way, can we get rid of appmenu-button-container in favor of
> titlebar-content?
> 

The container seems to be what's keeping the fx button from stretching the height of titlebar-content, and align="start" start doesn't have any effect on button.(Will play with it a bit to see.)

This brings up a question I had, what platforms have MENUBAR_CAN_AUTOHIDE defined? Related to that, should I remove this css:

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.css#89

or maybe move this css in my patch for winstripe:

#main-window[inFullscreen="true"] > #titlebar {..

over to base/content/browser.css?
(Assignee)

Comment 26

7 years ago
> Isn't the default button min-width always wrong, not just for classic?

Hmm, checking in dom inspector, it is actually. But the only place it seems to have an effect on the button widths is on classic. *shrug* I'll move these up though.
(In reply to comment #25)
> This brings up a question I had, what platforms have MENUBAR_CAN_AUTOHIDE
> defined?

Only Windows right now.

> Related to that, should I remove this css:
> 
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.css#89

Yes, it doesn't apply anymore.

> or maybe move this css in my patch for winstripe:
> 
> #main-window[inFullscreen="true"] > #titlebar {..
> 
> over to base/content/browser.css?

Yes.
(Assignee)

Comment 28

7 years ago
Created attachment 462504 [details]
stretch

(In reply to comment #25)
> (In reply to comment #24)
> The container seems to be what's keeping the fx button from stretching the
> height of titlebar-content, and align="start" start doesn't have any effect on
> button.(Will play with it a bit to see.)

We may want to stretch this actually, maybe to 50% or so, now that customizations are working right. I'll file a follow up.
(Assignee)

Comment 29

7 years ago
Created attachment 462873 [details] [diff] [review]
patch v.6

> Does your patch put the window controls in the tab order?

AFAICT from testing, no it doesn't. This seems to mimic the behavior when windows is handling this area. (I can't tab to the buttons, the focus moves from the tab to the content.)  

Other nits addressed.
Attachment #462434 - Attachment is obsolete: true
Attachment #462873 - Flags: review?(dao)
(Assignee)

Comment 30

7 years ago
I'll have try builds posted here in a few hours.
(Assignee)

Comment 31

7 years ago
Created attachment 462988 [details] [diff] [review]
patch v.6

Whoops, removed the moz-user-input junk.
Attachment #462873 - Attachment is obsolete: true
Attachment #462988 - Flags: review?(dao)
Attachment #462873 - Flags: review?(dao)
(Assignee)

Comment 32

7 years ago
FYI to drivers, this should block beta 4, not beta N.

Comment 33

7 years ago
Tryserver builds are available and I tested one. Great! Is there another bug for putting tabs into titlebar when the window is maximized or will that land with this bug?
Bug 572160
Can you show me link to tryserver build?

Comment 35

7 years ago
http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/jmathies@mozilla.com-719ed62bd8ac/
(Assignee)

Comment 36

7 years ago
Created attachment 463192 [details]
xp - zune

Honestly, I'm not a huge fan of the way the fx button looks on xp themes. It just doesn't seem to fit right. There's plenty of time to tweak the appearance once these patches land though. (shorlander, does this match up with what you have in the mockups?)
(Assignee)

Comment 37

7 years ago
Created attachment 463195 [details]
xp - standard
(Assignee)

Comment 38

7 years ago
Created attachment 463196 [details]
xp - classic
(In reply to comment #36)

> Honestly, I'm not a huge fan of the way the fx button looks on xp themes. It
> just doesn't seem to fit right. There's plenty of time to tweak the appearance
> once these patches land though. (shorlander, does this match up with what you
> have in the mockups?)

The plan is for the Firefox button to have a more XPish look that integrates with the custom window frame and buttons as seen here: https://wiki.mozilla.org/Firefox/4.0_Windows_Theme_Mockups#Windows_XP
(Assignee)

Comment 40

7 years ago
Created attachment 463206 [details]
win7 - aero basic
blocking2.0: betaN+ → beta4+
(In reply to comment #35)
> http://ftp.mozilla.org/pub/mozilla.org/firefox/tryserver-builds/jmathies@mozilla.com-719ed62bd8ac/

This actually appears to have focusable window controls. Maybe toolbarbuttons instead of buttons would work? Otherwise style="-moz-user-focus: ignore;" will probably do the job.
Comment on attachment 462988 [details] [diff] [review]
patch v.6

>-  document.getElementById("appmenu-button-container").hidden = !displayAppButton;
>-
>-  if (displayAppButton)
>+  if (displayAppButton) {
>     document.documentElement.setAttribute("chromemargin", "0,-1,-1,-1");
>-  else
>+    document.getElementById("titlebar").hidden = false;
>+  } else {
>     document.documentElement.removeAttribute("chromemargin");
>+    document.getElementById("titlebar").hidden = true;
>+  }

Less code churn if you just replace appmenu-button-container with titlebar.

> #appmenu-button-container {
>   -moz-binding: url("chrome://global/content/bindings/general.xml#windowdragbox");
> }

Remove this?

>+#titlebar {
>+  -moz-appearance:-moz-window-titlebar;

nit: space after colon

>+  /* we only need to the middle section, hide the edges of the
>+  theme background beyond the window frame. */
>+  margin-left: -15px;
>+  margin-right: -15px;
>+  -moz-binding: url("chrome://global/content/bindings/general.xml#windowdragbox");

The last line should probably move to browser/base/content/browser.css.

>+#titlebar-min {
>+  -moz-appearance: -moz-window-button-minimize;
>+  -moz-margin-start: 1px;
>+  -moz-margin-end: 1px;
>+  min-width: 0 !important;
>+}
>+
>+#titlebar-max {
>+  -moz-appearance: -moz-window-button-maximize;
>+  -moz-margin-start: 1px;
>+  -moz-margin-end: 1px;
>+  min-width: 0 !important;
>+}

>+#titlebar-close {
>+  -moz-appearance: -moz-window-button-close;
>+  -moz-margin-start: 1px;
>+  -moz-margin-end: 0;
>+  min-width: 0 !important;
>+}

!important shouldn't be necessary here. (You can remove min-width altogether when using toolbarbuttons.)

>+@media all and (-moz-windows-classic) {
>+  #main-window[sizemode="normal"] > #titlebar > #titlebar-content > #appmenu-button-container {
>+    margin-top: 0 !important;
>   }

Wrapping the part which added the margin-top with "@media not all and (-moz-windows-classic) {...}" (mind the "not") might be better.
(Assignee)

Comment 43

7 years ago
Created attachment 463329 [details] [diff] [review]
patch v.7

updated per comments.
Attachment #462988 - Attachment is obsolete: true
Attachment #463329 - Flags: review?(dao)
Attachment #462988 - Flags: review?(dao)
Comment on attachment 463329 [details] [diff] [review]
patch v.7

>+  if (window.windowState == window.STATE_MAXIMIZED) {
>+    window.restore();
>+  } else {
>+    window.maximize();
>+  }

nit: preferred style in this file is to omit the braces

>+#titlebar-min {
>+  -moz-appearance: -moz-window-button-minimize;
>+  -moz-margin-start: 1px;
>+  -moz-margin-end: 1px;
>+}
>+
>+#titlebar-max {
>+  -moz-appearance: -moz-window-button-maximize;
>+  -moz-margin-start: 1px;
>+  -moz-margin-end: 1px;
>+}
>+
>+#main-window[sizemode="maximized"] > #titlebar > #titlebar-content > #titlebar-buttonbox > #titlebar-max {
>+  -moz-appearance: -moz-window-button-restore;
>+  -moz-margin-start: 1px;
>+  -moz-margin-end: 1px;
>+}
>+
>+#titlebar-close {
>+  -moz-appearance: -moz-window-button-close;
>+  -moz-margin-start: 1px;
>+  -moz-margin-end: 0;
>+}
>+
>+/* windows classic titlebar treatment */
>+
>+@media all and (-moz-windows-classic) {
>+  #titlebar-min {
>+    -moz-margin-end: 0px !important;
>   }
>-  #navigator-toolbox[tabsontop="true"] > #toolbar-menubar[autohide="true"] ~ #TabsToolbar:not([inFullscreen]) {
>-    -moz-padding-start: 10em !important;
>+
>+  #titlebar-max {
>+    -moz-margin-start: 0px !important;
>   }
>-%ifdef WINSTRIPE_AERO
> }
>-%endif

Toolbarbuttons don't have a margin by default, which means that you should be able to remove the -moz-margin-*: 0 instances and wrap the 1px stuff to exclude classic like this:

@media not all and (-moz-windows-classic) {
  #titlebar-min,
  #titlebar-max {
    -moz-margin-start: 1px;
    -moz-margin-end: 1px;
  }

  #titlebar-close {
    -moz-margin-start: 1px;
  }
}

And if you add up the 1px margins you can simplify it further:

@media not all and (-moz-windows-classic) {
  #titlebar-min,
  #titlebar-max {
    -moz-margin-end: 2px;
  }
}
Attachment #463329 - Flags: review?(dao) → review+
Blocks: 581858
Using that tryserver build linked a few comments back, Minefield's titlebar is taller than normal when on Windows 7 with Aero enabled. Is it using XP's titlebar height for all platforms?
Created attachment 463446 [details]
Fx button and Tabs on Top Gap difference

Left side is Maximized mode, Right is a Normal window size.  The gap between fx button and tabs toolbar vary.
Blocks: 584955
(Assignee)

Comment 47

7 years ago
Created attachment 463538 [details]
normal w/notepad

The new titlebar "parts" on aero don't paint, but they are still present and take up space in the layout. The space is determined by system settings. The old titlebar didn't respect titlebar height metrics, the new one does. Attached is a comparison of the normal window before and after w/notepad. (If you max the browser and a notepad window and flip back and forth you'll see maximized state is also correct.) This is why these patch will also fix bug 576960, bug 578729, bug 581023, bug 582020, etc..
(Assignee)

Comment 48

7 years ago
Created attachment 463563 [details]
padding

There is a little bit of padding on top of the tabs we might take out. The bottom border, margin, and padding on the #titlebar and top border, margin, and padding on #navigator-toolbar are all zero though.

Dao any ideas on where this extra pixel or two of padding is coming from?
(In reply to comment #48)
> Created attachment 463563 [details]
> padding
> 
> There is a little bit of padding on top of the tabs we might take out. The
> bottom border, margin, and padding on the #titlebar and top border, margin, and
> padding on #navigator-toolbar are all zero though.
> 
> Dao any ideas on where this extra pixel or two of padding is coming from?

What exactly is the bottom red line? Is the top edge or the bottom edge of that line or the area in between of interest?
(Assignee)

Comment 50

7 years ago
(In reply to comment #49)
> (In reply to comment #48)
> > Created attachment 463563 [details] [details]
> > padding
> > 
> > There is a little bit of padding on top of the tabs we might take out. The
> > bottom border, margin, and padding on the #titlebar and top border, margin, and
> > padding on #navigator-toolbar are all zero though.
> > 
> > Dao any ideas on where this extra pixel or two of padding is coming from?
> 
> What exactly is the bottom red line? Is the top edge or the bottom edge of that
> line or the area in between of interest?

In dom inspector I highlighted #titlbar and snapped a screen shot.
(Assignee)

Comment 51

7 years ago
Looks like the padding below that is in question. If I highlight #navigator-toolbar seems like there's a small amount of space between the two boundaries.
This means that tabs have a one pixel padding at the top, which is expected... It's a very faint shadow, not quite transparent.
(Assignee)

Comment 53

7 years ago
(In reply to comment #51)
> Looks like the padding below that is in question. If I highlight
> #navigator-toolbar seems like there's a small amount of space between the two
> boundaries.

Sorry -> '#navigator-toolbox' not '#navigator-toolbar'.

(In reply to comment #52)
> This means that tabs have a one pixel padding at the top, which is expected...
> It's a very faint shadow, not quite transparent.

That would explain it. I'll test with tabs on bottom as well.
(Assignee)

Comment 54

7 years ago
Created attachment 463568 [details]
tabsonbottom w/notepad

The shadow looks to be the cause. With tabs on bottom, we line up with notepad perfectly.
(Assignee)

Comment 55

7 years ago
(In reply to comment #44)
> Comment on attachment 463329 [details] [diff] [review]
> patch v.7
> 
> >+  if (window.windowState == window.STATE_MAXIMIZED) {
> >+    window.restore();
> >+  } else {
> >+    window.maximize();
> >+  }
> 
> nit: preferred style in this file is to omit the braces
> 
> >+#titlebar-min {
> >+  -moz-appearance: -moz-window-button-minimize;
> >+  -moz-margin-start: 1px;
> >+  -moz-margin-end: 1px;
> >+}
> >+
> >+#titlebar-max {
> >+  -moz-appearance: -moz-window-button-maximize;
> >+  -moz-margin-start: 1px;
> >+  -moz-margin-end: 1px;
> >+}
> >+
> >+#main-window[sizemode="maximized"] > #titlebar > #titlebar-content > #titlebar-buttonbox > #titlebar-max {
> >+  -moz-appearance: -moz-window-button-restore;
> >+  -moz-margin-start: 1px;
> >+  -moz-margin-end: 1px;
> >+}
> >+
> >+#titlebar-close {
> >+  -moz-appearance: -moz-window-button-close;
> >+  -moz-margin-start: 1px;
> >+  -moz-margin-end: 0;
> >+}
> >+
> >+/* windows classic titlebar treatment */
> >+
> >+@media all and (-moz-windows-classic) {
> >+  #titlebar-min {
> >+    -moz-margin-end: 0px !important;
> >   }
> >-  #navigator-toolbox[tabsontop="true"] > #toolbar-menubar[autohide="true"] ~ #TabsToolbar:not([inFullscreen]) {
> >-    -moz-padding-start: 10em !important;
> >+
> >+  #titlebar-max {
> >+    -moz-margin-start: 0px !important;
> >   }
> >-%ifdef WINSTRIPE_AERO
> > }
> >-%endif
> 
> Toolbarbuttons don't have a margin by default, which means that you should be
> able to remove the -moz-margin-*: 0 instances and wrap the 1px stuff to exclude
> classic like this:
> 
> @media not all and (-moz-windows-classic) {
>   #titlebar-min,
>   #titlebar-max {
>     -moz-margin-start: 1px;
>     -moz-margin-end: 1px;
>   }
> 
>   #titlebar-close {
>     -moz-margin-start: 1px;
>   }
> }
> 
> And if you add up the 1px margins you can simplify it further:
> 
> @media not all and (-moz-windows-classic) {
>   #titlebar-min,
>   #titlebar-max {
>     -moz-margin-end: 2px;
>   }
> }

Trying to coalesce all this together. I'm not sure how I arrived at a -moz-margin-start for the min button though. I think that needs to come out.

Comment 56

7 years ago
Created attachment 463752 [details]
The positions of close/min/max buttons are not right in non native theme for xp

The positions of close/min/max buttons are not right in non native theme for xp
(Assignee)

Comment 57

7 years ago
(In reply to comment #56)
> Created attachment 463752 [details]
> The positions of close/min/max buttons are not right in non native theme for xp
> 
> The positions of close/min/max buttons are not right in non native theme for xp

Please file a new bug on this, after the patches land.
(Assignee)

Comment 58

7 years ago
http://hg.mozilla.org/mozilla-central/rev/a0d6e4d37273
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED

Comment 59

7 years ago
Created attachment 464286 [details]
screenshot

need more height on XP ?
(Assignee)

Comment 60

7 years ago
(In reply to comment #59)
> Created attachment 464286 [details]
> screenshot
> 
> need more height on XP ?

Possibly, I would post that in bug 574681.

Comment 61

7 years ago
(In reply to comment #60)
> (In reply to comment #59)
> > Created attachment 464286 [details] [details]
> > screenshot
> > 
> > need more height on XP ?
> 
> Possibly, I would post that in bug 574681.

OK, thanks.

Comment 62

7 years ago
when Firefox Button on by default for Windows XP, classic, and aero basic
like bug 574435 ?
(In reply to comment #62)
> when Firefox Button on by default for Windows XP

Never, as far as I remember.
Jim, any reason why you did this:

@media not all and (-moz-windows-classic) {
  #titlebar-min {
    -moz-margin-end: 1px;
  }

  #titlebar-max {
    -moz-margin-start: 1px;
    -moz-margin-end: 1px;
  }

  #titlebar-close {
    -moz-margin-start: 1px;
    -moz-margin-end: 0;
  }
}

instead of:

@media not all and (-moz-windows-classic) {
  #titlebar-min,
  #titlebar-max {
    -moz-margin-end: 2px;
  }
}

?

Updated

7 years ago
Depends on: 585924
(Assignee)

Comment 65

7 years ago
(In reply to comment #64)
> Jim, any reason why you did this:
> 
> @media not all and (-moz-windows-classic) {
>   #titlebar-min {
>     -moz-margin-end: 1px;
>   }
> 
>   #titlebar-max {
>     -moz-margin-start: 1px;
>     -moz-margin-end: 1px;
>   }
> 
>   #titlebar-close {
>     -moz-margin-start: 1px;
>     -moz-margin-end: 0;
>   }
> }
> 
> instead of:
> 
> @media not all and (-moz-windows-classic) {
>   #titlebar-min,
>   #titlebar-max {
>     -moz-margin-end: 2px;
>   }
> }
> 
> ?

I can post a patch. Shouldn't each button have it's own padding or does it make any difference?
The outcome should be identical.
This was backed out for the following performance regression:
Talos Regression: Txul increase 4.57% on Win7 Firefox

Other possible culprits:
bug 574454

http://hg.mozilla.org/mozilla-central/rev/9165ca3607bd
http://hg.mozilla.org/mozilla-central/rev/5704ef94d87f
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Updated

7 years ago
Blocks: 577197
(Assignee)

Updated

7 years ago
No longer blocks: 577197
(Assignee)

Updated

7 years ago
Blocks: 575036

Comment 68

7 years ago
When this was backed-out the window title bar is now no longer available (on WinXP SP3 x86). With tabs-on-top you can't grab anything to move the window. With tabs on the bottom (or on top) the window controls (minimize, resize/fullscreen, close) are not there. Must close browser with Minefield button>Exit or Alt-F4.
Pic available at
http://www.flickr.com/photos/9086763@N06/4883464958/
Yes, kind of my thinking as well.  If this is too remain backed out, then perhaps bug 574454needs to be backed out also otherwise if you have the browser set to autohide the taskbar, you end up with kind of a non-functional window.
(In reply to comment #68)
> When this was backed-out the window title bar is now no longer available (on
> WinXP SP3 x86). With tabs-on-top you can't grab anything to move the window.
> With tabs on the bottom (or on top) the window controls (minimize,
> resize/fullscreen, close) are not there. Must close browser with Minefield
> button>Exit or Alt-F4.
> Pic available at
> http://www.flickr.com/photos/9086763@N06/4883464958/

It is actually possible to move the window, athough fairly convoluted.  You have to right click on the taskbar icon and select move, then press an arrow key to move the window in any direction.  once you do that the mouse can be used to complete the move of the window to the desired location.
(Assignee)

Comment 71

7 years ago
(In reply to comment #69)
> Yes, kind of my thinking as well.  If this is too remain backed out, then
> perhaps bug 574454needs to be backed out also otherwise if you have the browser
> set to autohide the taskbar, you end up with kind of a non-functional window.

Hmm, we also removed compsitor checks to get ready for this landing in bug 574454. I think the only change set that needs to come out is:

http://hg.mozilla.org/mozilla-central/rev/05dde680ade2

I really don't have the bandwidth to do this and watch the tree today. Shawn, any chance you might be able to back this additional patch out?

Updated

7 years ago
No longer blocks: 582020

Updated

7 years ago
Blocks: 582231

Updated

7 years ago
Blocks: 587303
(In reply to comment #71)
> (In reply to comment #69)
> > Yes, kind of my thinking as well.  If this is too remain backed out, then
> > perhaps bug 574454needs to be backed out also otherwise if you have the browser
> > set to autohide the taskbar, you end up with kind of a non-functional window.
> 
> Hmm, we also removed compsitor checks to get ready for this landing in bug
> 574454. I think the only change set that needs to come out is:
> 
> http://hg.mozilla.org/mozilla-central/rev/05dde680ade2
> 
> I really don't have the bandwidth to do this and watch the tree today. Shawn,
> any chance you might be able to back this additional patch out?

done:

http://hg.mozilla.org/mozilla-central/rev/391e102eb863
http://hg.mozilla.org/mozilla-central/rev/cfeb77de159c
I take it this has no chance of making beta4?
blocking2.0: beta4+ → beta5+
(Assignee)

Updated

7 years ago
Blocks: 586228
(Assignee)

Updated

7 years ago
No longer blocks: 586228
(Assignee)

Comment 74

7 years ago
(In reply to comment #67)
> This was backed out for the following performance regression:
> Talos Regression: Txul increase 4.57% on Win7 Firefox
> 
> Other possible culprits:
> bug 574454
> 
> http://hg.mozilla.org/mozilla-central/rev/9165ca3607bd
> http://hg.mozilla.org/mozilla-central/rev/5704ef94d87f

Took some time today to get txul running locally to try and track this down. Oddly enough though I'm actually getting slightly better results locally with these patches applied. The difference is pretty minor, about 3 msec on my machine.

Will do some more testing tomorrow and see what I can come up with.
(In reply to comment #74)
> Will do some more testing tomorrow and see what I can come up with.

Can we use tryserver and mconnor's compare Talos tool at http://perf.snarkfest.net/compare-talos/
(Assignee)

Comment 76

7 years ago
(In reply to comment #75)
> (In reply to comment #74)
> > Will do some more testing tomorrow and see what I can come up with.
> 
> Can we use tryserver and mconnor's compare Talos tool at
> http://perf.snarkfest.net/compare-talos/

I can use try to confirm a fix, but not search for a fix. The lag on results is too high.
In any event, the patch has now bit-rotted.
(Assignee)

Comment 78

7 years ago
Created attachment 467780 [details] [diff] [review]
patches

If you're interested in playing with it, here's the two that were backed out, updated to latest trunk.
(Assignee)

Comment 79

7 years ago
So my first guess is this has to do with the clipped titlebar background image rendered on themed desktops:

+#titlebar {
+  -moz-appearance: -moz-window-titlebar;
+  /* we only need to the middle section, hide the edges of the
+  theme background beyond the window frame. */
+  margin-left: -15px;
+  margin-right: -15px;
+}

The reasoning for this is that we only want to render the middle section of the background, so the edges are pushed outside the rendering rectangle to clip them out.

Unfortunately this doesn't explain why xp *didn't* see the ~5msec increase. Our xp talos slaves use the default xp theme, which is no different than aero basic.

I'm still stumped.
Created attachment 468051 [details] [diff] [review]
patches v2

That patch did not really fix the issue of it not applying on current trunk.
Attachment #467780 - Attachment is obsolete: true
(In reply to comment #79)
> So my first guess is this has to do with the clipped titlebar background image
> rendered on themed desktops:
> 
> +#titlebar {
> +  -moz-appearance: -moz-window-titlebar;
> +  /* we only need to the middle section, hide the edges of the
> +  theme background beyond the window frame. */
> +  margin-left: -15px;
> +  margin-right: -15px;
> +}
> 
> The reasoning for this is that we only want to render the middle section of the
> background, so the edges are pushed outside the rendering rectangle to clip
> them out.
> 
> Unfortunately this doesn't explain why xp *didn't* see the ~5msec increase. Our
> xp talos slaves use the default xp theme, which is no different than aero
> basic.
> 
> I'm still stumped.

Could this have something to do with the desktop manager?

I believe that under Vista and Windows 7 even if you are running Aero basic in which case the Desktop Manger adds nothing other than overhead, it still runs.

Of course under Windows/XP it does not exist.
(Assignee)

Comment 82

7 years ago
(In reply to comment #81)
> (In reply to comment #79)
> > So my first guess is this has to do with the clipped titlebar background image
> > rendered on themed desktops:
> > 
> > +#titlebar {
> > +  -moz-appearance: -moz-window-titlebar;
> > +  /* we only need to the middle section, hide the edges of the
> > +  theme background beyond the window frame. */
> > +  margin-left: -15px;
> > +  margin-right: -15px;
> > +}
> > 
> > The reasoning for this is that we only want to render the middle section of the
> > background, so the edges are pushed outside the rendering rectangle to clip
> > them out.
> > 
> > Unfortunately this doesn't explain why xp *didn't* see the ~5msec increase. Our
> > xp talos slaves use the default xp theme, which is no different than aero
> > basic.
> > 
> > I'm still stumped.
> 
> Could this have something to do with the desktop manager?
> 
> I believe that under Vista and Windows 7 even if you are running Aero basic in
> which case the Desktop Manger adds nothing other than overhead, it still runs.
> 
> Of course under Windows/XP it does not exist.

I've been running some extended txul runs in vmware, painting of the titlebar doesn't appear to be the issue.
(Assignee)

Comment 83

7 years ago
Looks like the likely culprit is the button metrics query we do on Vista and up -

http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsNativeThemeWin.cpp#167

http://mxr.mozilla.org/mozilla-central/source/widget/src/windows/nsUXThemeData.cpp#273

vmware win7 txul runs:
trunk: 115
minus button query: 117
no painting: 115
fully applied: 121

I'll do some timing tests on these calls to see. If this is the case, there may not be much we can do about it. We need to make this call on Vista and up to get the right button dimensions.
(Assignee)

Comment 84

7 years ago
(In reply to comment #83)
> Looks like the likely culprit is the button metrics query we do on Vista and up
> -

Actually it's the changes to layout that takes place when we switch the titlebar buttons from collapsed to visible. The titlebar message we send to get the right metrics isn't the problem.

This regression did show up on xp according to txul results, it just wasn't as obvious.
(Assignee)

Comment 85

7 years ago
Created attachment 468349 [details] [diff] [review]
updates v.1

prevent using titlebar info calls on xp and vista/win7 w/aero glass. 

These are some improvements for the original patches, but they don't fix the regression on win7 aero basic, which is what our talos slaves run.
(Assignee)

Comment 86

7 years ago
Created attachment 468430 [details] [diff] [review]
catmgr experiment

Another experiment. This seemed to shave a little time off, but it still didn't run as fast as trunk.
(Assignee)

Updated

7 years ago
Attachment #463752 - Attachment is obsolete: true
(Assignee)

Comment 87

7 years ago
Created attachment 468497 [details] [diff] [review]
win experiment

bsmedberg suggested something like this. The window that gets created won't display on the desktop, but there's a small risk of it showing up very briefly on the taskbar on slower systems. FWIW, I didn't notice it in my vmware image while running txul.
(Assignee)

Updated

7 years ago
Attachment #468430 - Flags: feedback?(roc)
(Assignee)

Updated

7 years ago
Attachment #468497 - Flags: feedback?(roc)
(Assignee)

Comment 88

7 years ago
roc, the purpose here is to remove or minimize a 4-5 msec txul regression on aero basic and xp themed desktops. I'm leaning toward the win fix myself since it removed the need for collapsed content. Curious if you have any thoughts.
(Assignee)

Comment 89

7 years ago
Comment on attachment 468430 [details] [diff] [review]
catmgr experiment

Been testing a bit, I think I'll go with the win patch.
Attachment #468430 - Attachment is obsolete: true
Attachment #468430 - Flags: feedback?(roc)
(Assignee)

Updated

7 years ago
Attachment #468497 - Flags: feedback?(roc) → review?(roc)
(Assignee)

Updated

7 years ago
Attachment #468497 - Flags: review?(dao)
Could you use the hidden window here instead of creating one?

I assume the ShowWindow call is necessary here?

It seems like we should be able to stop it showing up in the taskbar, using the right combination of styles. Dialog boxes don't usually show up in the taskbar, for example.
(Assignee)

Comment 91

7 years ago
(In reply to comment #90)
> Could you use the hidden window here instead of creating one?

The hidden message window actually has a bit of code tied to it that prevents it from being shown. (in OnWindowPosChanging) I guess at some point a 3rd party app was messing with it. So I went with my own window/class to avoid any conflict. I could pull or trap that bit of code. Since this new window is created just once at the start of a session, I just went with my own isolated (non-nsWindow subclassed) window.

> 
> I assume the ShowWindow call is necessary here?

Yes, the window has to be set to the visible state or the titlebar metrics aren't correct.

> 
> It seems like we should be able to stop it showing up in the taskbar, using the
> right combination of styles. Dialog boxes don't usually show up in the taskbar,
> for example.

Tried that. I used ws_ex_toolwindow. But the metrics values were different from a standard overlapped window.
(Assignee)

Comment 92

7 years ago
(In reply to comment #91)
> (In reply to comment #90)
> > 
> > It seems like we should be able to stop it showing up in the taskbar, using the
> > right combination of styles. Dialog boxes don't usually show up in the taskbar,
> > for example.
> 
> Tried that. I used ws_ex_toolwindow. But the metrics values were different from
> a standard overlapped window.

Looked at this a bit more. It's true some dialogs do not show up, but these are modal children. There might be a way to trick windows on this, although since I have no visible parent, it might not be possible. I'll play with it a bit more tomorrow.
Comment on attachment 468497 [details] [diff] [review]
win experiment

>-    <hbox id="titlebar-buttonbox" collapsed="true">
>+    <hbox id="titlebar-buttonbox" collapsed="false">

nit: remove collapsed="false"

> @media all and (-moz-windows-compositor) {
>+  /* these should be hidden w/glass enabled. windows draws it's own buttons. */
>+  #titlebar-min,
>+  #titlebar-max,
>+  #titlebar-close {
>+    visibility: hidden;
>+  }

Does widget code somehow depend on visibility:hidden or would display:none work as well?
Duplicate of this bug: 584955
(Assignee)

Updated

7 years ago
Depends on: 590146
(Assignee)

Comment 95

7 years ago
Comment on attachment 468497 [details] [diff] [review]
win experiment

Updated patch coming. By passing a parent and creating a stand alone child, even if the parent is hidden, we can still get the right values. This should fix the taskbar problem.

Dao, I'll update to display:none and put the other button css changes in as well in the follow up.
Attachment #468497 - Flags: review?(roc)
Attachment #468497 - Flags: review?(dao)
(Assignee)

Comment 96

7 years ago
Created attachment 468715 [details] [diff] [review]
updates v.2
Attachment #468349 - Attachment is obsolete: true
Attachment #468497 - Attachment is obsolete: true
(Assignee)

Updated

7 years ago
Attachment #468715 - Flags: review?(roc)
(Assignee)

Updated

7 years ago
Attachment #468715 - Flags: review?(dao)
(Assignee)

Updated

7 years ago
Blocks: 590146
No longer depends on: 590146
Comment on attachment 468715 [details] [diff] [review]
updates v.2

> @media all and (-moz-windows-compositor) {
>+  /* these should be hidden w/glass enabled. windows draws it's own buttons. */
>+  #titlebar-min,
>+  #titlebar-max,
>+  #titlebar-close {
>+    display: none;
>+  }

Just hide #titlebar-buttonbox, unless -moz-appearance: -moz-window-button-box; is doing anything useful here?
Attachment #468715 - Flags: review?(dao) → review+
Comment on attachment 468715 [details] [diff] [review]
updates v.2

It seems crazy that we need to create and show a window to get these metrics, but if Windows gives us no choice, so mote it be!
Attachment #468715 - Flags: review?(roc) → review+

Updated

7 years ago
No longer blocks: 590146
(Assignee)

Comment 99

7 years ago
http://hg.mozilla.org/mozilla-central/rev/323eb3b2eb2a
Status: REOPENED → RESOLVED
Last Resolved: 7 years ago7 years ago
Resolution: --- → FIXED
(Assignee)

Updated

7 years ago
No longer blocks: 582231

Comment 100

7 years ago
The title doesn't refresh for me with maximized window (Win7 Aero basic, b5pre 20100824 but may already exist in older versions). See screenshot:
http://img714.imageshack.us/img714/9802/ffaerobasic.png
(Assignee)

Comment 101

7 years ago
(In reply to comment #100)
> The title doesn't refresh for me with maximized window (Win7 Aero basic, b5pre
> 20100824 but may already exist in older versions). See screenshot:
> http://img714.imageshack.us/img714/9802/ffaerobasic.png

Please check the 08/25 nightly when it's out in a bit. This work just landed late last night.

Comment 102

7 years ago
(In reply to comment #0)
Looking at the mockups it looks like the title bar will no longer display the page name, is that accurate?

Are we meant to have an empty blue space at the top of the window and don't actually see the complete page name if it overflows the tab width unless we hover the pointer on the tab? =S

Even Microsoft Office's "ribbon" shows the document name at the top.
(In reply to comment #102)
> (In reply to comment #0)
> Looking at the mockups it looks like the title bar will no longer display the
> page name, is that accurate?
> 
> Are we meant to have an empty blue space at the top of the window and don't
> actually see the complete page name if it overflows the tab width unless we
> hover the pointer on the tab? =S
> 
> Even Microsoft Office's "ribbon" shows the document name at the top.

I believe your looking for bug 575487.

Comment 104

7 years ago
(In reply to comment #103)
> I believe your looking for bug 575487.
Thanks! But that's sadly already RESOLVED WONTFIX
Bug 583905 seems to be my only hope.

Comment 105

7 years ago
(In reply to comment #101)
> (In reply to comment #100)
> > The title doesn't refresh for me with maximized window (Win7 Aero basic, b5pre
> > 20100824 but may already exist in older versions). See screenshot:
> > http://img714.imageshack.us/img714/9802/ffaerobasic.png
> 
> Please check the 08/25 nightly when it's out in a bit. This work just landed
> late last night.

cannot check.
with 0825 Nightly, titlebar is gone.

Updated

7 years ago
Depends on: 590880

Updated

7 years ago
No longer depends on: 590880
(In reply to comment #105)
> cannot check.
> with 0825 Nightly, titlebar is gone.

It sounds like you might not be using the default Firefox theme.  The new menu and the Firefox button are currently only supported in the default theme (and maybe a handful of others fi some themers are really on-the-ball keeping up with beta releases).

If you are using a theme other than the Firefox default, please try the default theme.

Other themes should start supporting the new UI features as the release date approaches.
The other thing is that some extensions and userchrome.css changes would likely interfere.  I know of at least one extension in particular that exhibits the behavior pal-moz mentioned.

Comment 108

7 years ago
I meant title is not displayed with Firefox Button ON.(Menu Bar OFF)
I know title is displayed with Firefox Button OFF.(Menu Bar ON)
(In reply to comment #108)
> I meant title is not displayed with Firefox Button ON.(Menu Bar OFF)
> I know title is displayed with Firefox Button OFF.(Menu Bar ON)

Oh you said the titlebar was not displayed.  I guess what you meant was that the page title is no longer displayed int he titlebar.
(Assignee)

Comment 110

7 years ago
(In reply to comment #109)
> (In reply to comment #108)
> > I meant title is not displayed with Firefox Button ON.(Menu Bar OFF)
> > I know title is displayed with Firefox Button OFF.(Menu Bar ON)
> 
> Oh you said the titlebar was not displayed.  I guess what you meant was that
> the page title is no longer displayed int he titlebar.

bug 583905.
Target Milestone: --- → Firefox 4.0b5

Updated

7 years ago
Depends on: 592185
(Assignee)

Updated

7 years ago
No longer depends on: 592185

Updated

7 years ago
Depends on: 592185

Updated

7 years ago
Depends on: 594371

Updated

7 years ago
No longer depends on: 594371

Updated

6 years ago
No longer blocks: 576960
You need to log in before you can comment on or make changes to this bug.