#1667 Fix macOS 11.0 Big Sur issues

Closed
opened 7 months ago by dbsoft · 9 comments
dbsoft commented 7 months ago (Migrated from github.com)

There are 3 things which I am working on... two are able to be committed now, but was trying to fix all three before making a set of commits/patches:

  1. _pthread_self() undefined in jemalloc.c
    In the DARWIN section I just #define _pthread_self() pthread_self() which seems to be the correct fix based on updated jemalloc.c code in Mozilla and other sources.

  2. Add OnBigSurOrLater() to nsCocoaFeatures.h/mm and add sections in the theme to draw things that match the 11.0 Big Sur look and feel.

  3. Fix popup menus not drawing correctly. Almost all popup/dropdown menus in Pale Moon fail to draw correctly... the text and separators are all missing. However the highlight color appears when you move the mouse over them and clicking them works properly, you just can’t see the text to know what you are clicking on.

Comparing to the behavior in Basilisk, the popups in Australis that are more complicated render properly, like the search field and the menu button, function properly but the more basic menus when you secondary/right click on a page appear blank like in Pale Moon.

It is clear that it is drawing the menus itself, and not using native menus. In nsNativeThemeCocoa.mm there are methods to draw the menu background and highlight which appears to be working as expected, it is just the menu text and separators that are missing, but I haven’t yet identified where in the code this is (or should be) happening.

There are 3 things which I am working on... two are able to be committed now, but was trying to fix all three before making a set of commits/patches: 1) _pthread_self() undefined in jemalloc.c In the DARWIN section I just #define _pthread_self() pthread_self() which seems to be the correct fix based on updated jemalloc.c code in Mozilla and other sources. 2) Add OnBigSurOrLater() to nsCocoaFeatures.h/mm and add sections in the theme to draw things that match the 11.0 Big Sur look and feel. 3) Fix popup menus not drawing correctly. Almost all popup/dropdown menus in Pale Moon fail to draw correctly... the text and separators are all missing. However the highlight color appears when you move the mouse over them and clicking them works properly, you just can't see the text to know what you are clicking on. Comparing to the behavior in Basilisk, the popups in Australis that are more complicated render properly, like the search field and the menu button, function properly but the more basic menus when you secondary/right click on a page appear blank like in Pale Moon. It is clear that it is drawing the menus itself, and not using native menus. In nsNativeThemeCocoa.mm there are methods to draw the menu background and highlight which appears to be working as expected, it is just the menu text and separators that are missing, but I haven't yet identified where in the code this is (or should be) happening.
dbsoft commented 5 months ago

Since fixing the initial list of problems on this issue, another problem has been identified, OpenGL is failing to load, so WebGL and accelerated rendering is not available. Part of the issue is that the libraries are failing to load, there is a fix in NSPR 4.27 that addresses this referenced in this Mozilla bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1652330

However with this patch an assert in the Pale Moon GL rendering subsystem gets hit so I am not including that in the first pull request, a second pull request will be coming when I sort out the assert.

Since fixing the initial list of problems on this issue, another problem has been identified, OpenGL is failing to load, so WebGL and accelerated rendering is not available. Part of the issue is that the libraries are failing to load, there is a fix in NSPR 4.27 that addresses this referenced in this Mozilla bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1652330 However with this patch an assert in the Pale Moon GL rendering subsystem gets hit so I am not including that in the first pull request, a second pull request will be coming when I sort out the assert.
Moonchild added the
Wanted: Release Uplift
label 5 months ago
Moonchild commented 5 months ago
Owner

Marking this as uplift wanted because of Big Sur’s final release.

Marking this as uplift wanted because of Big Sur's final release.
Moonchild commented 5 months ago
Owner

Were you going to make a second PR re-doing the commits for it, or...?

Were you going to make a second PR re-doing the commits for it, or...?
dbsoft commented 5 months ago

Don’t think I need a second PR, there was only one addition to nsChildView.mm adding the main thread check in the draw handler that differed from the part 2 commit, to avoid the OpenGL related assert. So seems to make sense for that change to be in Part 3. What do you think?

Don't think I need a second PR, there was only one addition to nsChildView.mm adding the main thread check in the draw handler that differed from the part 2 commit, to avoid the OpenGL related assert. So seems to make sense for that change to be in Part 3. What do you think?
Moonchild commented 5 months ago
Owner

I’m fine with the PR as it is.

I'm fine with the PR as it is.
dbsoft commented 5 months ago

Recapping a private message conversation with Moonchild, after applying the NSPR fix to load OpenGL, Pale Moon would hit a main thread assert in nsObserverService.cpp. I disabled this assert and Pale Moon seemed to function properly with full OpenGL support, but I was confused as to why this assert was getting hit on MacOS 11 but no other versions. Tobin had said he wouldn’t merge the other patches without the OpenGL fix so I asked Moonchild if I could disable this assert just for MacOS until I figured it out. Moonchild was not in favor of this solution due to possible race condition crashes which could cause security problems, and asked if it was any of the changes I made causing this. So I did a build with only the jemalloc and NSPR changes, and that build also hit a main thread assert in a similar manner, although a slightly different code path. I went back to searching and after looking at some other Mozilla forks I noticed that they had added a main thread check in the draw/paint callback due to CoreAnimation related race conditions from callbacks on the non-main thread. I wondered if this check was preventing the same sort of race condition stemming from the fact that OpenGL on MacOS 11 seems to generate a draw event on the calling thread when setting up the OpenGL surface. So I added the main thread check that Mozilla was using in the draw event callback and tested with the original nsObserverService.cpp and everything worked.

Recapping a private message conversation with Moonchild, after applying the NSPR fix to load OpenGL, Pale Moon would hit a main thread assert in nsObserverService.cpp. I disabled this assert and Pale Moon seemed to function properly with full OpenGL support, but I was confused as to why this assert was getting hit on MacOS 11 but no other versions. Tobin had said he wouldn't merge the other patches without the OpenGL fix so I asked Moonchild if I could disable this assert just for MacOS until I figured it out. Moonchild was not in favor of this solution due to possible race condition crashes which could cause security problems, and asked if it was any of the changes I made causing this. So I did a build with only the jemalloc and NSPR changes, and that build also hit a main thread assert in a similar manner, although a slightly different code path. I went back to searching and after looking at some other Mozilla forks I noticed that they had added a main thread check in the draw/paint callback due to CoreAnimation related race conditions from callbacks on the non-main thread. I wondered if this check was preventing the same sort of race condition stemming from the fact that OpenGL on MacOS 11 seems to generate a draw event on the calling thread when setting up the OpenGL surface. So I added the main thread check that Mozilla was using in the draw event callback and tested with the original nsObserverService.cpp and everything worked.
dbsoft commented 5 months ago

I did notice one more oddity on Big Sur, it isn’t anything too major, but the UserAgent is showing the MacOS version as 10.0, instead of 11.0 or 10.16 as expected. Anyone know whereabouts in the code it sets that? It does not seem to use the code in nsCocoaFeatures.mm

I did notice one more oddity on Big Sur, it isn't anything too major, but the UserAgent is showing the MacOS version as 10.0, instead of 11.0 or 10.16 as expected. Anyone know whereabouts in the code it sets that? It does not seem to use the code in nsCocoaFeatures.mm
Moonchild commented 5 months ago
Owner

Please make a new issue out of the useragent oddity, as it’s not a release blocker.

Please make a new issue out of the useragent oddity, as it's not a release blocker.
Moonchild closed this issue 5 months ago
Moonchild commented 5 months ago
Owner

Uplifted in [53ca366d2], [4f0f4aa87] and [08cc9cd55]

Uplifted in [53ca366d2], [4f0f4aa87] and [08cc9cd55]
Moonchild removed the
Wanted: Release Uplift
label 5 months ago
Sign in to join this conversation.
No Milestone
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

This issue currently doesn't have any dependencies.

Loading…
There is no content yet.