The look and feel system is hardcoded to return fixed values for the eMetric_ScrollButtonXXXMouseButtonAction values; on GTK2 they are always 0, 1 and 2 while on other platforms they are always 0, 3 and 3. Either the hardcoded values should be moved to nsScrollbarButtonFrame.cpp or the metrics should be fixed to use the look and feel system correctly.
Created attachment 230421 [details] [diff] [review] hardcode values Not shown are the backouts of the changes to look and feel.
Created attachment 230427 [details] [diff] [review] correct usage
I really can't review these changes. FWIW, roc was the one who advised me to make these changes in order to fix the win32 bustage. As roc mentioned, we really don't want to allow people to change these prefs. so I think that using the ifdefs for GTK2 would be OK here.
As I don't have GTK2 could you at least test that the patch you prefer works?
Created attachment 230472 [details] [diff] [review] hardcode values
I don't know why you feel that hardcoding execution paths with #ifdefs is preferable to using nsILookAndFeel. I think it's much more ugly and less extensible. The difference in complexity is marginal.
In particular, with the "hardcoded values" patch, if we ever want to support any other behaviour for any button on any other platform, the code will become a complete mess.
(In reply to comment #7) >In particular, with the "hardcoded values" patch, if we ever want to support >any other behaviour for any button on any other platform, the code will become >a complete mess. Well I guess I could write it slightly differently: #ifdef MOZ_WIDGET_GTK2 #define LEFTBUTTONACTION pressedButtonAction = 0 #define MIDDLEBUTTONACTION pressedButtonAction = 1 #define RIGHTBUTTONACTION pressedButtonAction = 2 #else #define LEFTBUTTONACTION pressedButtonAction = 0 #define MIDDLEBUTTONACTION return PR_FALSE #define RIGHTBUTTONACTION return PR_FALSE #endif and leave most of the existing scrollbarbutton code in place.
I don't think that's an improvement, honestly. And I don't know why you find using nsILookAndFeel so upsetting here.
(In reply to comment #9) >I don't know why you find using nsILookAndFeel so upsetting here. Because those particular metrics don't work like any of the existing metrics.
nsILookAndFeel is an interface. The new settings fit nicely into it. The fact that they're implemented differently shouldn't matter, in my opinion.
You need to log in before you can comment on or make changes to this bug.