Bug 182711

Created attachment 95727 [details]
stacktrace

100% reproducible, and makes the IDE completely unusable.  Same bug happens in J2SE projects - since many project types have copy/pasted EncodingRenderer (I know I did it for Java Card projects), probably this needs to be fixed in many places.

Also appears in
java.lang.AssertionError
	at org.netbeans.modules.javacard.project.customizer.CustomizerSources$EncodingRenderer.getListCellRendererComponent(CustomizerSources.java:220)
	at com.apple.laf.AquaComboBoxButton.doRendererPaint(AquaComboBoxButton.java:147)
	at com.apple.laf.AquaComboBoxButton.paintComponent(AquaComboBoxButton.java:139)
	at javax.swing.JComponent.paint(JComponent.java:1029)

java.lang.AssertionError
	at org.netbeans.modules.java.j2seproject.ui.customizer.CustomizerSources$EncodingRenderer.getListCellRendererComponent(CustomizerSources.java:254)
	at com.apple.laf.AquaComboBoxButton.doRendererPaint(AquaComboBoxButton.java:147)
	at com.apple.laf.AquaComboBoxButton.paintComponent(AquaComboBoxButton.java:139)
	at javax.swing.JComponent.paint(JComponent.java:1029)

In JDK 6, sometimes (String) " " is passed as a value to list cell renderers rather than an object of the expected type. You can either handle this special value, or set a prototypeDisplayValue for the control. See bug #180088 for discussion. (Normally this occurs because you switch to GroupLayout but I think it could happen for other layout managers too.)

And slightly different variant in freeform, ruby and web projects - the problem
is, re-clicking the current value in a Mac OS combo box will cause one paint
with a null value.

java.lang.NullPointerException
    at
org.netbeans.modules.java.freeform.ui.SourceFoldersPanel$EncodingRenderer.getListCellRendererComponent(SourceFoldersPanel.java:1066)
    at
com.apple.laf.AquaComboBoxButton.doRendererPaint(AquaComboBoxButton.java:147)
    at
com.apple.laf.AquaComboBoxButton.paintComponent(AquaComboBoxButton.java:139)
    at javax.swing.JComponent.paint(JComponent.java:1029)
  
Patch in progress...

Given that two variants on the same buggy code appear in many different project types, would be nice to have a factory method for an encoding renderer in some utility class - this is silly.

(In reply to comment #6)
> would be nice to have a factory method for an encoding renderer in some
> utility class

Agreed, please file an RFE marked API for projectuiapi.

Created attachment 95731 [details]
Patch

Patch adding ProjectCustomizer.createEncodingRenderer() and ProjectCustomizer.createEncodingModel(String encoding), deleting all copy/pasted variants of encoding models and renderers and changing C/C++, EJB, Java SE, Java Card, Maven, PHP, Python, Ruby and Web projects to use these factory methods.

Would like *not* to wait out the normal API-review-fast period, as this change should be completely uncontroversial and is a textbook case of where an API is quite obviously needed - 9 copies of the same buggy code all needing bug fixing = missing API.  ProjectCustomizer is the most intuitive place available for such factory methods.

(In reply to comment #9)
> Would like *not* to wait out the normal API-review-fast period

Reviewers must be given the chance to review the patch. The grace period is seven days except for P1 bugs.

> this change should be completely uncontroversial

That remains to be seen.

[JG01] You are misusing spec.version.base, e.g. in cnd.makeproject:

-spec.version.base=1.10.0
+spec.version.base=1.10.1

The last component should always be zero in trunk:

http://wiki.netbeans.org/DevFaqImplementationDependency

Should be:

-spec.version.base=1.10.0
+spec.version.base=1.11.0

Similarly in other places.


[JG02] In j2ee.ejbjarproject, you use the new API, but fail to delete the old impls.


[JG03] Don't forget @since, apichanges.xml.


[JG04] "create" prefix on factory methods is generally superfluous; obviously something is being created, this is not news. Could just use e.g. "encodingRenderer()", "encodingModel(String)". (Compare Collections.synchronizedSet and so on.)


[JG05] setIcon(null) is unnecessary since it was already null. getListCellRendererComponent could be simplified to just:

if (value instanceof Charset) {
  value = ((Charset) value).displayName();
}
return super.getListCellRendererComponent(list, value, index, isSelected, isLeadSelection);

> [JG01] You are misusing spec.version.base, e.g. in cnd.makeproject:

Was puzzling over what it meant if something's spec version was 1.41.0 and its implementation version was 6 (as with projectuiapi). Can't say I understand how that would interact with incrementing versions in branch builds (1.41.6.1? 1.41.7? 1.4.0.6? 1.4.0.7?) or what it means that some modules use two decimal places and others use 3. But I digress...

> Reviewers must be given the chance to review the patch. The grace period is
> seven days except for P1 bugs.

I believe the process is that if you get explicit approval from 1-2 reviewers, then you are clear, but if not, you should really wait for seven days. Thanks for this change.

[off-topic]

(In reply to comment #12)
> Was puzzling over what it meant if something's spec version was 1.41.0 and its
> implementation version was 6 (as with projectuiapi). Can't say I understand how
> that would interact with incrementing versions in branch builds (1.41.6.1?
> 1.41.7? 1.4.0.6? 1.4.0.7?)

1.41.1.6 (from s.v.b=1.41.1); all explained in FAQ, I think.

> what it means that some modules use two decimal places and others use 3

Explicit OIDE-M-S-V in manifest.mf should always be x.y, s.v.b in project.properties should always be x.y.0 in trunk.

Created attachment 95874 [details]
Updated patch

New patch should address all comments

Created attachment 95875 [details]
Patch for contrib modules

Created attachment 95974 [details]
Final patch (fixes a typo in java.freeform/project.xml, no code changes)

Fixed in main/ 1e820140c2db

[JG06] I think the impl is java.freeform is wrong; should call ProjectCustomizer.encodingModel with model.getEncoding(), perhaps? Surely using Charset.defaultCharset().name() as the initial value is not right.

Freeform fixed in main/ 1bff24f8886f

I noticed that the php module, which has several encoding combo boxes, all use the default charset.  Looks incorrect, but I did not delve into it.

(In reply to comment #20)
> I noticed that the php module, which has several encoding combo boxes, all use
> the default charset.  Looks incorrect, but I did not delve into it.

Where exactly? Could you please clarify/file issues? Thanks.

The PHP module had a public class Utils.EncodingModel with one constructor that took a String for the project's character set, and a no-argument constructor that called this(null).  A null value would result in Charset.defaultCharset() being used.

All calls that constructed a Utils.EncodingModel used the no-arg constructor - so all encoding combo boxes in PHP projects were initialized with the default character set.  So I replaced Utils.EncodingModel with ProjectCustomizer.encodingModel(Charset.defaultCharset().name()).  

Maybe there is some initialization code that sets the value after model creation - I didn't look for it, I just patched it to have the same behavior it already had, using ProjectCustomizer.encodingModel(). 

I just noticed that, at least at construction time, the model's value was not initialized from the project, which seemed strange.

(In reply to comment #22)
> Maybe there is some initialization code that sets the value after model
> creation

Right, the correct value is set when the wizard panel is displayed (method readSettings()), the encoding is detected:

    private Charset getEncoding() {
        Charset enc = (Charset) descriptor.getProperty(ENCODING);
        if (enc == null) {
            // #136917
            enc = FileEncodingQuery.getDefaultEncoding();
        }
        return enc;
    }

Backing out the contrib #6c4c67fa73ed patch in contrib #dfd25eec0acb until the new API propagates to main-silver. Tim please reapply it when that happens.

Integrated into 'main-golden', will be available in build *201004020200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/1e820140c2db
User: tboudreau@netbeans.org
Log: #182711 - 16 copies (including contrib/) of the same buggy encoding renderer code removed