Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use System default Look And Feel (especially Feel) #2227

Closed
asfimport opened this issue Apr 21, 2009 · 12 comments
Closed

Use System default Look And Feel (especially Feel) #2227

asfimport opened this issue Apr 21, 2009 · 12 comments

Comments

@asfimport
Copy link
Collaborator

Frank Caputo (Bug 47064):
I patched JMeter to use the system look and feel by default and allow ctrl-click on the mac and use the apple-key for keyboard shortcuts of menu items.

All changes are platform independent.
Instead of KeyEvent.CTRL_DOWN_MASK use Toolkit.getDefaultToolkit().getMenuShortcutKeyMask().

isRightClick also checks e.isPopupTrigger().

attached is the patchfile for 2.3.2.

Created attachment jmeter.diff: svn patch file

OS: All

@asfimport
Copy link
Collaborator Author

Sebb (migrated from Bugzilla):
Thanks for the patch.

Unfortunately, the patch does not seem to work for me - most of the changes are rejected. I don't know why that is.

Also, there is a Mac-specific change to jmeter.sh - are you sure that won't affect non-Mac Unixes? Can you generate a patch that detects the Mac?

@asfimport
Copy link
Collaborator Author

Frank Caputo (migrated from Bugzilla):
(In reply to comment 1)

Thanks for the patch.

no problem.

Unfortunately, the patch does not seem to work for me - most of the changes are
rejected. I don't know why that is.

for me the patch works:

svn co http://svn.apache.org/repos/asf/jakarta/jmeter/tags/v2_3_2
cd v2_3_2
patch -p0 -i ~/Desktop/jmeter.diff
ant

ant compiles fine without errors.

Also, there is a Mac-specific change to jmeter.sh - are you sure that won't
affect non-Mac Unixes? Can you generate a patch that detects the Mac?

i simply set a system property which is simply ignored on other systems than the mac.

@asfimport
Copy link
Collaborator Author

Sebb (migrated from Bugzilla):
It works for me against the tag; however the current code in trunk has moved on a lot since then, which is presumably why the patch does not work against it.

I suppose I can just do the replacements manually.

If I create a new nightly build with the changes, would you be able to test it out? I don't have a Mac to test on.

@asfimport
Copy link
Collaborator Author

Sebb (migrated from Bugzilla):
The System LAF on Windows is not as nice as the Metal LAF, and I'm not prepared to change all the LAFs just to support Macs.

I think one solution would be to allow the property "jmeter.laf" to have a local system override, e.g. "jmeter.laf.<os.name>" where <os.name> is obtained from the system at run-time.

JMeter would look for "jmeter.laf.<os.name>", then "jmeter.laf".

This would allow one to change the default just for Macs.

What does Java return for os.name on Macs?
This is shown in the jmeter.log file.

@asfimport
Copy link
Collaborator Author

Sebb (migrated from Bugzilla):
Applied most of the changes:

URL: http://svn.apache.org/viewvc?rev=768370&view=rev
Log:
#2227 - fixes for Mac LAF

These are in the nightly builds starting with r768370.

Please can you try it out, and post details of the os.name.

@asfimport
Copy link
Collaborator Author

@milamberspace (migrated from Bugzilla):
With New Revision 768370, JMeter doesn't start on Linux (ubuntu 9.04), because os.name is "linux" without space like "windows xp"

jmeter.log :
2009/04/24 20:23:38 FATAL - jmeter.JMeter: An error occurred: java.lang.ExceptionInInitializerError
at java.lang.Class.forName0(Native Method)
at java.lang.Class.forName(Class.java:169)
at org.apache.jmeter.gui.action.ActionRouter.populateCommandMap(ActionRouter.java:275)
at org.apache.jmeter.gui.action.ActionRouter.getInstance(ActionRouter.java:308)
at org.apache.jmeter.JMeter.startGui(JMeter.java:220)
at org.apache.jmeter.JMeter.start(JMeter.java:343)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at org.apache.jmeter.NewDriver.main(NewDriver.java:207)
Caused by: java.lang.NullPointerException
at org.apache.jmeter.gui.action.LookAndFeelCommand.getJMeterLaf(LookAndFeelCommand.java:77)
at org.apache.jmeter.gui.action.LookAndFeelCommand.<clinit>(LookAndFeelCommand.java:49)
... 11 more

Created attachment proposal_patch_update_47064.txt: Patch to work with os.name = "linux"

proposal_patch_update_47064.txt
Index: /Workspaces-JMeter/Jmeter/src/core/org/apache/jmeter/gui/action/LookAndFeelCommand.java
===================================================================
--- /Workspaces-JMeter/Jmeter/src/core/org/apache/jmeter/gui/action/LookAndFeelCommand.java	(revision 768370)
+++ /Workspaces-JMeter/Jmeter/src/core/org/apache/jmeter/gui/action/LookAndFeelCommand.java	(working copy)
@@ -73,8 +73,8 @@
         if (laf != null) {
             return laf;
         }
-        String osFamily = osName.substring(0, osName.indexOf(' '));// e.g. windows xp => windows
-        laf = JMeterUtils.getProperty(JMETER_LAF+"."+osFamily);
+        String[] osFamily = osName.split("\\s"); // e.g. windows xp => windows
+        laf = JMeterUtils.getProperty(JMETER_LAF+"."+osFamily[0]);
         if (laf != null) {
             return laf;
         }

@asfimport
Copy link
Collaborator Author

@milamberspace (migrated from Bugzilla):
Note: I don't know if "System.getProperty("os.name")" always return a correct string, but if it's return null, there are a possible NPE.

@asfimport
Copy link
Collaborator Author

Sebb (migrated from Bugzilla):
Thanks for report and patch, applied in:

URL: http://svn.apache.org/viewvc?rev=768414&view=rev
Log:
Oops - avoid NPE if os.name has no space in it.
Props to Milamber, see #2227

I'll update the nightly build shortly.

I don't think os.name should ever return null ...

@asfimport
Copy link
Collaborator Author

Frank Caputo (migrated from Bugzilla):
(In reply to comment 4)

The System LAF on Windows is not as nice as the Metal LAF, and I'm not prepared
to change all the LAFs just to support Macs.

I think one solution would be to allow the property "jmeter.laf" to have a
local system override, e.g. "jmeter.laf.<os.name>" where <os.name> is obtained
from the system at run-time.

JMeter would look for "jmeter.laf.<os.name>", then "jmeter.laf".

This would allow one to change the default just for Macs.

What does Java return for os.name on Macs?
This is shown in the jmeter.log file.

Mac OS X returns "Mac OS X" for "os.name".

I think you should do something like:

-------- code ------------------------------------

// make os.name lower case and replace alt whitespaces with underscores
String osName = System.getProperty("os.name").toLowerCase().replaceAll("\s", "_");
String laf = System.getProperty("jmeter.laf");

String osLaf = System.getProperty("jmeter.laf." + osName);
if(osLaf != null) {
laf = osLaf;
}

-------- code ------------------------------------

Here are some interesting links for os.name:
http://developer.apple.com/technotes/tn2002/tn2110.html
http://mindprod.com/jgloss/properties.html#OSNAME

I will test the next nightly.

@asfimport
Copy link
Collaborator Author

Sebb (migrated from Bugzilla):
I think this is now finally fixed.

I've added the ability to set the LAF to System or CrossPlatform, as well as to individual LAF implementations:

URL: http://svn.apache.org/viewvc?rev=770777&view=rev
Log:
#2227 - Use System default Look And Feel (Mac)

Please re-open if the nightlies after r770777 don't work as expected on a Mac.

@asfimport
Copy link
Collaborator Author

Frank Caputo (migrated from Bugzilla):
I tried r772972 and it works fine out of the box on my mac.

@asfimport
Copy link
Collaborator Author

Sebb (migrated from Bugzilla):
Great - thanks for confirming this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant