If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

VibrancyManager NSView recycling doesn't update correctly when BackingScaleFactor changes

RESOLVED FIXED in Firefox 44

Status

()

Core
Widget: Cocoa
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: BenWa, Assigned: BenWa)

Tracking

(Blocks: 1 bug)

42 Branch
mozilla44
Points:
---

Firefox Tracking Flags

(firefox44 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
One of the sub problems of bug 1205372 is that the OSX Vibrancy effect doesn't update until you resize the window if you drag it from a 1x monitor to a 2x monitor.

I tracked it down to the logic for recycling the views but I'm not sure why it doesn't work because we compute the same rect regardless if we create or recycle a view.
(Assignee)

Updated

2 years ago
Assignee: nobody → bgirard
(Assignee)

Comment 1

2 years ago
Created attachment 8668136 [details]
MozReview Request: Bug 1210180 - Force the view to update when we recycle a Vibrancy view. r=mstange

Bug 1210180 - Force the view to update when we recycle a Vibrancy view. r=mstange
Attachment #8668136 - Flags: review?(mstange)
Comment on attachment 8668136 [details]
MozReview Request: Bug 1210180 - Force the view to update when we recycle a Vibrancy view. r=mstange

https://reviewboard.mozilla.org/r/20893/#review19109

I like this patch much better!

Interestically, I can't reproduce this bug on 10.11. And theoretically, frame changes should always cause an NSView to be marked as needing display - in other places we've added workarounds to suppress needsDisplays for frame changes that don't need to invalidate. So maybe Apple have fixed a bug here.

::: widget/cocoa/VibrancyManager.mm:38
(Diff revision 1)
> +        view.needsDisplay = YES;

Let's make this \[view setNeedsDisplay:YES\] to be consistent with setFrame. And let's call setFrame first and setNeedsDisplay afterwards (assuming that still works), I think that would make more sense to the reader.

At some point I should go through widget/cocoa and make us convert more stuff to property syntax instead of setter syntax, but let's not start an inconsistent mix now.
Attachment #8668136 - Flags: review?(mstange) → review+
(Assignee)

Comment 3

2 years ago
Comment on attachment 8668136 [details]
MozReview Request: Bug 1210180 - Force the view to update when we recycle a Vibrancy view. r=mstange

Bug 1210180 - Force the view to update when we recycle a Vibrancy view. r=mstange
(Assignee)

Comment 4

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/21a11ea9774895d651943e27e94e487ad027a089
Bug 1210180 - Force the view to update when we recycle a Vibrancy view. r=mstange
https://hg.mozilla.org/mozilla-central/rev/21a11ea97748
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
You need to log in before you can comment on or make changes to this bug.