Bug 47064 - Use System default Look And Feel (especially Feel)
Use System default Look And Feel (especially Feel)
Status: RESOLVED FIXED
Product: JMeter
Classification: Unclassified
Component: Main
2.3.2
All All
: P5 enhancement (vote)
: ---
Assigned To: JMeter issues mailing list
: PatchAvailable
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2009-04-21 08:19 UTC by Frank Caputo
Modified: 2009-05-14 08:54 UTC (History)
1 user (show)



Attachments
svn patch file (19.51 KB, patch)
2009-04-21 08:19 UTC, Frank Caputo
Details | Diff
Patch to work with os.name = "linux" (838 bytes, patch)
2009-04-24 13:36 UTC, Milamber
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Frank Caputo 2009-04-21 08:19:09 UTC
Created attachment 23522 [details]
svn patch file

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.
Comment 1 Sebb 2009-04-23 16:11:07 UTC
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?
Comment 2 Frank Caputo 2009-04-24 01:10:33 UTC
(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.
Comment 3 Sebb 2009-04-24 04:28:52 UTC
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.
Comment 4 Sebb 2009-04-24 05:32:10 UTC
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.
Comment 5 Sebb 2009-04-24 10:13:36 UTC
Applied most of the changes:

URL: http://svn.apache.org/viewvc?rev=768370&view=rev
Log:
Bug 47064 - 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.
Comment 6 Milamber 2009-04-24 13:36:52 UTC
Created attachment 23541 [details]
Patch to work with os.name = "linux"

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
Comment 7 Milamber 2009-04-24 13:37:51 UTC
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.
Comment 8 Sebb 2009-04-24 13:53:09 UTC
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 Bug 47064

I'll update the nightly build shortly.

I don't think os.name should ever return null ...
Comment 9 Frank Caputo 2009-04-27 01:14:19 UTC
(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.
Comment 10 Sebb 2009-05-01 11:12:58 UTC
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:
 Bug 47064 -  Use System default Look And Feel (Mac)

Please re-open if the nightlies after r770777 don't work as expected on a Mac.
Comment 11 Frank Caputo 2009-05-14 08:02:14 UTC
I tried r772972 and it works fine out of the box on my mac.
Comment 12 Sebb 2009-05-14 08:54:19 UTC
Great - thanks for confirming this.