This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 248664 - HighlightingManager does not track changes to HighlightsLayerExcludes correctly
Summary: HighlightingManager does not track changes to HighlightsLayerExcludes correctly
Status: REOPENED
Alias: None
Product: editor
Classification: Unclassified
Component: Painting & Printing (show other bugs)
Version: 7.4
Hardware: PC Mac OS X
: P4 normal with 1 vote (vote)
Assignee: Miloslav Metelka
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-11-15 00:52 UTC by ebakke
Modified: 2018-04-22 19:28 UTC (History)
2 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:

Attachments
Proposed patch (1.61 KB, application/octet-stream)
2016-07-07 19:23 UTC, ebakke 		Details
Re-uploading proposed patch with correct MIME type/marked as "patch" (1.61 KB, patch)
2016-10-24 20:20 UTC, ebakke 		Details | Diff
Testing code for manually testing the HighlightingManager patch. (1.23 KB, text/plain)
2016-10-24 22:27 UTC, ebakke 		Details
Show Obsolete (1) Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description ebakke 2014-11-15 00:52:54 UTC 
The editor API defines the JEditorPane client properties HighlightsLayerExcludes and HighlightsLayerIncludes [1] to allow clients to show/hide specific highlight layers on demand. The bug is that modifications to these properties are not reflected properly in the editor.

The bug seems to be in o.n.modules.editor.lib2.highlighting.HighlightingManager.Highlighting. In the constructor, the paneFilter variable is initialized once from the HighlightsLayerExcludes/HighlightsLayerIncludes client properties, right above where the propertyChangeListener that is supposed to track changes to it is added. The PropertyChangeListener reacts to changes to the client properties by calling rebuildAllLayers(), which calls rebuildAllLayersImpl(), which always just uses the value of paneFilter set in the constructor.

A simple fix is to set paneFilter again in the PropertyChangeListener prior to calling rebuildAllLayers(), or to set it to null from the PropertyChangeListener and move initialization from the constructor to rebuildAllLayers (to avoid duplicating the "new RegExpFilter" logic).

[1] http://bits.netbeans.org/dev/javadoc/org-netbeans-modules-editor-lib2/index.html?org/netbeans/spi/editor/highlighting/package-summary.html
Comment 1 Martin Balin 2016-07-07 07:29:44 UTC 
This old bug may not be relevant anymore. If you can still reproduce it in 8.2 development builds please reopen this issue.

Thanks for your cooperation,
NetBeans IDE 8.2 Release Boss
Comment 2 ebakke 2016-07-07 19:23:17 UTC 
Created attachment 160306 [details]
Proposed patch

Proposed patch to fix this bug.
Comment 3 ebakke 2016-07-07 19:26:43 UTC 
The bug still seems to be present in the latest sources. Should be easy enough to fix; I've attached a proposed patch.
Comment 4 markiewb 2016-10-24 20:07:54 UTC 
@Ebakke: Please mark your patch as patch! Now it is only an octet-stream
Comment 5 ebakke 2016-10-24 20:20:48 UTC 
Created attachment 162616 [details]
Re-uploading proposed patch with correct MIME type/marked as "patch"
Comment 6 markiewb 2016-10-24 20:24:53 UTC 
@ebakke: Thanks. Looks simple. But how can I reproduce an issue within a current NB installation? I won't fix anything, which isn't broken. Especially when it is not my main topic.
Comment 7 ebakke 2016-10-24 22:27:00 UTC 
Created attachment 162621 [details]
Testing code for manually testing the HighlightingManager patch.

The attached patch shows how CloneableEditor.java can be temporarily modified to allow the previously attached HighlightManager patch to be tested manually.

To test the patch:
1) In the development environment, apply the patch in this attachment. This will change CloneableEditor to allow highlighting in the NetBeans code editor to be toggled on and off with the shift key.
2) Build and start the development version of the IDE and open up a Java file for editing. While the cursor is in the code editor, try pressing the shift key a couple of times. Nothing happens, because HighlightingManager does not currently react to changes in the HighlightsLayerExcludes property.
3) Now apply the proposed HighlightingManager patch, rebuild the editor.lib2 module, and restart the development version of the IDE.
4) Again, open up a Java file for editing. With the cursor in the code editor, again try pressing the shift key a couple of times. This now correctly toggles syntax highlighting in the code editor.
5) Remove the testing code from CloneableEditor before committing.
Comment 8 ebakke 2016-10-24 22:30:42 UTC 
This bug does not affect the IDE as far as I know, but affects my NetBeans Platform application [1]. My platform application features a spreadsheet-like "formula bar" that uses the NetBeans editor infrastructure to provide syntax highlighting. The use case is to be able to deactivate the highlighting (specifically, brace matching) when the component is not focused. This requires the HighlightingManager to properly respond to changes in the HighlightsLayerExcludes/HighlightsLayerIncludes properties.

[1] http://people.csail.mit.edu/ebakke/sieuferd
Comment 9 ebakke 2017-02-26 03:07:49 UTC 
I'd still love to see this patch applied--it's a blocker for a feature in my NetBeans Platform application.
Comment 10 emi 2017-02-28 21:20:07 UTC 
I don't know this class very much but I wonder if there is a reason we don't create a RegExpFilter each time we need it?

I mean, is the Pattern.compile call so expensive that we have to store it? Is rebuildAllLayersImpl called that often?

Anyhow, the patch is very small, it is correct (as we are respecting contract wrt to the PROP_HL_* properties) and I don't believe it can impact the IDE very much -- worse case scenario the IDE re-creates the same RegExpFilter in some very rare situations.

I believe we should apply the patch.
Comment 11 ebakke 2017-02-28 22:12:59 UTC 
Thanks for taking a look! The only reason I was caching the RegExpFilter was because the original code was doing so, so it seemed the option least likely to break anything. I have no strong opinions on this otherwise.
Comment 12 ebakke 2018-04-22 19:28:25 UTC 
Added as JIRA issue https://issues.apache.org/jira/browse/NETBEANS-713 .