blueboxd / chromium-legacy

Latest Chromium (≒Chrome Canary/Stable) for Mac OS X 10.7+
BSD 3-Clause "New" or "Revised" License
307 stars 17 forks source link

Title bar & min/max/close buttons #4

Closed wicknix closed 3 years ago

wicknix commented 3 years ago

Hey thanks for doing this project. I noticed the min/max/close buttons are missing and the full screen button is only partially shown. We had this same issue a while back with Pale Moon on Mac. Ended up having to revert the removal of the lion theme ( https://hg.mozilla.org/mozilla-central/rev/c3eb99e7b16d ) to get them back. Chrome code is alien to me, but i hope after looking at that link you might be able to make some sense of it on the chrome side, as it may be a similar issue.

blueboxd commented 3 years ago

Sorry for late reply. Thank you so much for your cooperation!

Chromium code is still strange to me, and window related UI code structure is drastically changed between version for 10.9 and for 10.10+. So, I should forward-port old window UI code to latest Chromium, that is not so easy work. Please wait for a while till this is done. Sorry for inconvenience.

Wowfunhappy commented 3 years ago

As a temporary stop-gap measure, this dumb code mostly fixes the problem.

@implementation NSWindow

-(void)update {

    NSButton* closeButton = [self standardWindowButton:NSWindowCloseButton];
    NSButton* miniaturizeButton = [self standardWindowButton:NSWindowMiniaturizeButton];
    NSButton* zoomButton = [self standardWindowButton:NSWindowZoomButton];
    NSButton* fullScreenButton = [self standardWindowButton:NSWindowFullScreenButton];

    [closeButton setFrameOrigin:NSMakePoint(closeButton.frame.origin.x, self.frame.size.height - 19)];
    [miniaturizeButton setFrameOrigin:NSMakePoint(miniaturizeButton.frame.origin.x, self.frame.size.height - 19)];
    [zoomButton setFrameOrigin:NSMakePoint(zoomButton.frame.origin.x, self.frame.size.height - 19)];
    [fullScreenButton setFrameOrigin:NSMakePoint(fullScreenButton.frame.origin.x, self.frame.size.height - 20)];

    [[[self contentView] superview] viewDidEndLiveResize];
}

@end

I'm just repositioning the buttons so they appear in the non-covered area of the toolbar.

Screen Shot 2021-01-27 at 10 15 49 PM

blueboxd commented 3 years ago

Wow!Thank you very much for your idea! I'll implement your idea at window initialization sequence. Please wait a while.

Wowfunhappy commented 3 years ago

I'll implement your idea at window initialization sequence.

Just a heads up, that may not be enough, because the buttons like to revert to their original positions whenever the window is resized. So you will probably need to detect that.


What I'm still annoyed about is that I couldn't figure out how to actually change the height of the window toolbar. It's too damn tall (albeit normally covered by the window's contentView), and I suspect that's the core problem.

Screen Shot 2021-01-25 at 10 57 58 PM

blueboxd commented 3 years ago

Thank you again! Implemented as update method as you write in latest build.

Wowfunhappy commented 3 years ago

Hi again! I actually had the opportunity to hack on this a bit more today, and I think I have a much better, permanent fix!

Before I go any further, please let me apologize for:

If you're busy, there's certainly no hurry to implement any of this—I can't make use it myself until that Mavericks font issue is resolved, anyway.

That said: as I suspected, the actual problem was the height and position of that title bar, which turns out to be controlled in NSThemeFrame. All I had to do was:

@implementation NSThemeFrame

- (double)_topBarHeight {
    return 22;
}

- (struct CGRect)titlebarRect {
    return NSMakeRect(self.frame.origin.x, self.frame.size.height - 22, self.frame.size.width, 22);
}

@end

This is the result:

Screen Shot 2021-01-29 at 8 17 04 PM

Unlike last time, the buttons won't jump around while resizing the window. It's not quite how Chrome is supposed to look, but it doesn't feel broken anymore, and IMO it actually makes Chrome's flat UI feel a bit less alien.


There is one thing that is still really bothering me—if Chromium is going to have that gray Cocoa title bar, it really ought to show the page title! Something is setting NSWindow's - (BOOL)_isTitleHidden method to return true, and I couldn't figure out how to change it via code injection.

But, I did do a quick search of Chromium's source code, and I think there's a good chance the culprit is here: https://github.com/chromium/chromium/blob/dabc4a8b666ddcb5c7cff5ed4ee04a938613b524/extensions/shell/browser/shell_native_app_window_mac.mm#L36. I would recommend flipping that to return false. If it doesn't work, I'll keep experimenting...

blueboxd commented 3 years ago

Thank you so much again for the great key! Window has now default classic style title bar on 10.9- in latest build. This should almost resolve this issue (except for nostalgic old-school UI).

FYI, actual _titlebarHeight is located at BrowserWindowFrame, and _isTitleHidden is at NativeWidgetMacNSWindow.

blueboxd commented 3 years ago

This issue should be considered solved by a simple classical title bar. Implement a unified title bar & tab strip should be a new issue as enhancement.

krackers commented 1 year ago

@Wowfunhappy I was taking a look at what it would take to get the unified titlebar style. Seems the issue was introduced with the native cocoa -> macviews redesign, as even on chrome 65, if you force enable macviews flag, then the titlebar is messed up.

Take a particular look at https://bugs.chromium.org/p/chromium/issues/detail?id=425229

They have screenshots of the progress, and the one at Screen Shot 2015-03-18 at 1.39.01 PM.png shows one with a separate titlebar, and one at Screen Shot 2015-03-30 at 12.38.06 PM.png shows one with a unified titlebar.

It seems https://chromium.googlesource.com/chromium/src.git/+/fd0ea6990e2c25537ee0b708d1ce7eeafc61842a was the key change. However I don't see anything that would actually be 10.10 specific there,

krackers commented 1 year ago

Aha I think it's https://source.chromium.org/chromium/chromium/src/+/3401663175b3a9803095bd43434d6886b6c2b378 and https://source.chromium.org/chromium/chromium/src/+/87ffcf9b935cc134c2dd8ecd4ebeb400eae03042 that broke it on 10.9?

Particularly the use of NSFullSizeContentViewWindowMask (now called NSWindowStyleMaskFullSizeContentView). Online I see a few mentions of workarounds, but we could also just use the technique they were doing before.

Possibly a separate issue is also https://bugs.chromium.org/p/chromium/issues/detail?id=520373 although I don't think that is related to the issue since it was always gated on >= 10.11