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.
Development build #200212110100 of NetBeans 4.0 Windows 2000 with FCS build #21 of JDK 1.4.1. Description: ============ The look of original "Basic" node in "Variables Editor" is really ugly. This is regression and causes automatic tests to fail. Steps to reproduce: =================== 1. Invoke "Versioning|Mount Version Control|Generic VCS". 2. Select CVS profile from combo box and push "Next >". 3. Click "Edit Variables..." button. 4. Look at the first tree subnode.
What happened with the ability of Nodes to interpret html??
Please don't rely on Nodes to display names in html. We don't want to use html renderer in JTree for performance reasons. Node's display name must be plain text. It can be used/displayed in many places not only in a JTree with JLabel as the cell renderer.
So, Trung, does that mean that we will not be able to have nodes of different color? That's a pity. Don't you plan to implement some hooks how to change at least the color? (It would be nice to be able to set a different font as well.)
> So, Trung, does that mean that we will not be able to have nodes of > different color? No, we don't plan this feature
There was a long discussion of this on nbdev... of interest: - pnejedly knows about patch which disabled HTML display names (regression, no plans to revert) - tboudreau started thread proposing new lighter-weight API using some kind of rendering hints - jglick proposed curses-like escape sequences - tball proposed ability to override a paint method for label
I didn't say we don't want to do it, just stated the fact that I don't know when this will happen
Please don't expect NetBeans to interpret html code in the Controls. Especially for JTree (TreeView) in explorer this has significant disadvantages in the area of performance. (The same is valid for changing font). In the future the JTree should use LargeModel (in order to prevent creation of all subnodes on node's expand). Using large model requires having all items (nodes) fixed height, which is impossible when changing fonts or rendering HTML. Some API for changing colors would be possible but is not planned and would require all views to interpret this. (The bug in VCS has to be solved different way) Changing this bug to a RFE.
Please note that color changes, as well as <b>, <i>, etc., require no change in font height and are thus compatible with a fixed label height. I don't think anyone ever *wanted* tree node labels with nonstandard heights. Is there no straightforward way to permit typical HTML to be rendered but to fix the line height to that of the base font (i.e. not support any markup which would change line height)?
Probably should be reassigned to Tim, mark STARTED for 4.0.
Status update: HTML branch exists, but is fragmented - cvs update -f html_may2003 I will put together a patch containing its contents, making sure it's buildable against the current trunk. Open issue (easily resolvable, I think): How to indicate html content. IMO there are two solutions: 1. Require html tag at beginning of string 2. Provide separate method, for nodes, et. al., getHTMLDisplay name. Returns null if no html version needed. IMO solution 2 is much more desirable. Reasons: - HTML tags don't need to be included. This is useful for composing strings (as a composite Look will want to do) without having to manage opening/closing HTML tags. - Code that can't deal with HTML doesn't have to (logging, process output, any non-UI-oriented use case) - Code that *wants* to use raw HTML/XML tags (such as an XML tree) doesn't need to do any special escaping of them (which will cause other problems when used for process output, etc.) Unacceptable solution: Just assume strings are HTML. This can fail in too many ways, cause problems in that display names are not just used for UI display (a better method name for Node.getDisplayName() might be Node.getHumanReadableName(), which would make the distinction clear. Reasons for disliking using opening HTML tags: - Still requires escaping by any object that manipulates the display name but doesn't want HTML - The idea of spending cpu cycles (even a few) generating HTMLized text, and further down the pipeline, spending more cycles stripping those tags out is evil. An assembly programmer would be shot for such code; sometimes it's better to think like an assembly programmer. Hrebejk, important question for you: How should this be handled in Looks? Other question: Choice of method name? - getHTMLDisplayName() or - getFormattedDisplayName() I think I like getFormattedDisplayName better. Makes it clearer that the full HTML spec is not necessarily supported.
Note also, new lightweight html renderer is already used in two places: - New winsys - for displaying tabs (so things like font face of tabs can be manipulated in order to indicate status) - Navigator module (private copy I'd like to delete)
Attaching a diff with pretty much final HTML rendering code. One thing: The rendering methods in Utilities have commented out assertions to make sure they aren't called off the AWT thread. I couldn't rid of 1.4 source warnings and get it to compile in openide - either we're not using source=1.4, or there's some ant property I couldn't track down. When/if we can compile with assertions, these lines should be uncommented.
Created attachment 11431 [details] HTML support for explorer patch
Created attachment 11432 [details] Example patch for javacvs module to use HTML annotations (looks pretty!)
You can freely include assertions in openide code. I just added source="1.4" to some <javac> tasks in the build script yesterday.
Tim, cvs diff -u please! Contextless diffs are unpleasant to read and unsafe to apply. Request that the new Utilities methods be moved to a separate static class. Yarda can give his opinion here - it seems that Utilities is getting more and more cluttered with all sorts of unrelated stuff, and it is very difficult to deprecate or refactor things when it is all mashed in the same class. If there is a block of conceptually related utility methods, they should go into their own class. DataNode.getFormattedDisplayName says "@since 1.73" which is nonsense - (1) should use the API spec version number, not some CVS revision; (2) DataNode is now in the Loaders API, not the general Open APIs, therefore there should be an independent spec version incr for openide-loaders.jar as well as an independent entry in the change list for that API. Ditto FileSystem, etc. - @since tags should refer to API spec versions. DataNode.gFDN Javadoc mentions U.renderString; should be renderHTML (right?). At least the @see tag says rHTML. There are outstanding "XXX" comments. Either fix whatever they refer to, or write what is actually still broken. Don't let future generations of programmers have to guess what is wrong with this code. VisualizerNode.getFDN() is a new API method, right? Should have @since, mention in apichanges. Ditto FilterNode, Node.PROP_F_D_N. Utilities methods and constants are missing @since too - though if you put all this stuff into one rendering utils class as I suggest, it suffices to have just one @since in the class Javadoc. Assertions may be uncommented, as I mentioned. Any method requiring AWT-only access should start with assert EventQueue.isEventDispatchThread(); Some parameters in Utilities methods missing Javadoc tags. Please run PMD, or Tasklist's suggestions, or AutoComment, or whatever, and check for violations. [Javadoc only?] Numeric character entities: "≶" not "&8822;". System property 'netbeans.lwhtml.strict': (1) better to use ErrorManager-compatible logging I think; (2) this prop must be documented in some */arch/arch-*.xml file. Re.: ---%<--- ErrorManager.getDefault().log(ErrorManager.WARNING, msg); System.err.println(msg); //Also print to stdout - ErrorManager warning will be cut off after first /n ---%<--- Huh? ErrorManager logs are not cut off after newlines. Please delete the serr; should not be there in production code. Re. String out = "Could not find color identifier in font declaration"; - need NOI18N decl. (In various other places, too.) Use of IllegalArgumentException - fine, but mention it in the throws clause of relevant methods, and include it in @throws tags in the Javadoc, describing when it may be thrown. <class package="org.openide.filesystems.FileSystem" name="HTMLStatus"/> is forbidden syntax for apichanges (bogus package); use <class package="org.openide.filesystems" name="FileSystem"/> (there is no support for listing changes in nested classes).
Created attachment 11462 [details] Test comparing swing and our HTML rendering
Reassigning this issue to myself (mainly because I'm tired of having to search for it when I need to update it), setting umbrella keyword and changing to type TASK for consistency with similar issues. It looks like we'll want a similar getHTMLDisplayName() for property objects - they are used in places (like dialog titles) where they shouldn't be html-ized. Propertysheet now disables HTML for the same reasons as Explorer. One thought: We seem to have a lot of patterns for ways to get a human-readable description of a property. To wit: - Node.getName() - well, not really human readable, but descriptive - Node.getDisplayName() - human readable name - Node.getHTMLDisplayName() - part of the branch - Node.getShortDescription() - Some hint (I don't remember what) for properties to supply a longer display name for dialogs - Other code to get (possibly html-ized) titles for editor tabs - similar methods for Property objects We have a proliferation of contexts in which different textual descriptions of an object are required, and a corresponding proliferation of methods to get those descriptions. I'm wondering if it might be better to simply have some well defined keys and a describeThyself() method, i.e. Node.getDescription(CONSTANT_INDICATING_DISPLAY_NAME_OR_WHATEVER) etc. It keeps the whole thing more flexible for the long term, and IMO is much less cluttered (and you can have reasonable fallback mechanisms without worrying that some subclass will, say, mangle displayName and result in a mangled HTMLDisplayName if they don't override that). What do you think?
I agree that we need to handle this consistently for all nodes and properties - and that a new method should be used for providing html, not misusing getDisplayName() (see e.g. my last comments in issue 11783). Having parametrized getDescription method is IMO good idea.
As of today this issue blocks 7 other issues. Seeing how we're redesigning the UI, it might be a good idea to take a look at this (especially since this issue is over a year old). Please consider upgrading its priority.
increasing priority doensn't help much. It's about resource allocation. Please consider contributing
FYI, I just created the following branch: html_yet_again for core openide java vcscore vcscvs form Will try to respond to Jesse and Jarda's comments, and get everything pretty and working there.
FYI, the branch is now functional, and there's a patch to VCSCore which uses the controlShadow color for status annotations. I've migrated all of the explorer views to use it, as well as the property sheet name and set renderers, the property sheet combo box cell editor and the new window system tabs and popup tab selection dialog. A border or two may be a pixel or two off, but it's mostly tweaking left to do - the major stuff is there. The open question is the nature of the API changes involved. I'm adding two classes to the APIS: org.openide.awt.HTMLRenderer - has the same static html painting methods as in earlier revisions, but is also a JLabel subclass which implements TableCellRenderer/TreeCellRenderer/ListCellRenderer, so that anyone wanting HTML rendering for such a control can simply override getDefaultRenderer() to return HTMLRenderer.sharedInstance() (each call to sharedInstance reinitializes the state). org.openide.util.Describable - a generic interface for requesting a human readable description. Provides one relevant method - getDescription (int key), and defines a few constants for plain/html description, etc., and bitmasks for indicating plaintext vs. html. There are three reasons to do this: - We don't want to be adding getHTMLDisplayName(), getHTMLShortDescription(), then maybe later getWeightedHTMLDisplayName, getEBDICHtmlDisplayName(), ... and generate a lot of clutter... - I do think we should leave the door open for other types of annotated/formatted strings. In particular, this would be the right mechanism for doing Navigator-style abbreviations - the client would weight characters by importance, the view would figure out what it could display and display that. Also, we have various magical hints like "longerDisplayName" and assorted gunk that would be better served by a more understandable API. - Provides an unambiguous way to indicate exactly what the client wants. How exactly we go about implementing it is manageable; the current branch implements Describable directly on Node, Node.PropertySet and Node.Property. I'm anticipating Jarda or someone will object to that - hence there is Describable.Provider (sort of a 1-method Look), which could just as easily be substituted and passed one of our myriad FeatureDescriptor subclasses. So, it'd be good to get some feedback on the API stuff as soon as possible. BTW, I still need to test it to make IconView works correctly - does anything use that these days?
Well, after the interlude of issue 38412, the branch is functional again, and is modified to reflect the feedback from nbdev, but is postponed until after 3.6.
Since there were tons of changes this week because of feature freeze, I've updated the html branch from the trunk, so everything is up to date and happy once again. I've added treefs to the branch, so HTML annotations are propagated from underlying CVS filesystems.
I just brought the changes up to date with the current trunk. I seem to have gotten some gunk committed to the html_yet_again branch, and my attempts to weed the gunk out ran smack into pretty massive "flexible build" changes in the trunk, so I'll just attach a diff with the current changes that I assembled against a clean check-out. Two things need further work: 1. (easy to fix) - The look for cell renderers is not exactly the same because the background extends under the icon, which is not what happens in trees in the trunk. 2. There is an issue if a node uses HTML, but the color specified in HTML does not contrast well with the selection color for trees, etc. In practice, windows classic L&F is the only place I know it will be a problem, since it uses a very dark blue with white text for selection, and black on white otherwise. Several ways to solve this: - Sidestep the look and feel for all trees/lists/tables and simply make sure the selection color is always one close enough in brightness to the default window background that this will never be a problem. - Extend logical colors specifiable in HTML to be something that is interpreted on the fly depending on the selected state of the renderer. This means we would need colors usable in HTML that have special logical meaning, i.e., say, font color=d1...d4 where 1-4 are varying levels of deemphasis, and do something similar for other logical uses of it. Comments?
Created attachment 13915 [details] diff against current trunk less the htmlrenderer class itself
Created attachment 13916 [details] org.openide.awt.HtmlRenderer
Okay, since the old branch was fairly mangled, I'm creating a new branch - html_renderer - base tag is BLD200403281800. With any luck and some reading, perhaps my branch management skills have improved. I'll check some stuff into in the next day or so, once I resolve any issues resulting from the Stavbicka merge.
Okay, I've improved the API of it to deal with some past objections, and gotten rid of any cases of subclassing. Here's a skeleton of how it looks now. The actual code can be found on the branch ui_cleanup_with_html (a subbranch merging html_renderer and ui_cleanup2), and soon on the html_renderer branch): public final class HtmlRenderer { /** Returns a shared cell renderer instance */ public static final Renderer sharedRenderer(); /** Actually returns the same thing as sharedRenderer(), but as a JLabel - we have a few * embedded renderer cases that need this */ public static final JLabel sharedLabel(); public interface Renderer extends TreeCellRenderer, TableCellRenderer, ListCellRenderer { /** Indicate that the component being rendered has keyboard focus. NetBeans requires that a different * selection color be used depending on whether the view has focus. * * @param parentFocused Whether or not the focused selection color should be used */ void setParentFocused (boolean parentFocused); /** * Indicate that the text should be painted centered below the icon. This is primarily used * by org.openide.explorer.view.IconView * * @param centered Whether or not centered painting should be used. */ void setCentered (boolean centered); /** * Set a number of pixels the icon and text should be indented. Used by ChoiceView and ListView to * fake tree-style nesting. This value has no effect if <code>setCentered(true)</ code> has been called. * * @param pixels The number of pixels to indent */ void setIndent (int pixels); /** * Explicitly tell the renderer it is going to receive HTML markup, or it is not. If the renderer should * check the string for opening HTML tags to determine this, don't call this method. If you <strong>know</strong> * the string will be compliant HTML, it is preferable to call this method with true; if you want to intentionally * render HTML markup literally, call this method with false. * * @param val */ void setHtml (boolean val); /** * Set the rendering style - this can be JLabel-style truncated-with-elipsis (...) text, or clipped text. * The default is STYLE_CLIP. * * @param style The text style */ void setRenderStyle (int style); /** Set the icon to be used for painting * * @param icon An icon or null */ void setIcon (Icon icon); /** Clear any stale data from previous use by other components, * clearing the icon, text, disabled state and other customizations, returning the component * to its initialized state. This is done automatically when get*CellRenderer() is called. */ void reset(); /** Set the text to be displayed. Use this if the object being rendered's toString() does not * return a real user-displayable string, after calling get**CellRenderer(). Typically after calling * this one calls setHtml() if the text is known to either be or not be HTML markup. * * @param txt The text that should be displayed */ void setText (String txt); /** * Convenience method to set the gap between the icon and text. * * @param gap an integer number of pixels */ void setIconTextGap (int gap); } public static String renderString (...); //same as before public static String renderHtml (...); //same as before }
Created attachment 14607 [details] More-or-less final diff
Why this commit contains code that is "FOR DEMO PURPOSES ONLY!". I do not like it. org.openide.awt.HtmlLabelUI was not in original plans or am I wrong? This should not be in the diff I guess: +/* + * HtmlRenderer.java + * + * Created on January 2, 2004, 12:49 AM + */ Please point me to the list of usecases that justify existence of HtmlRenderer.Renderer - btw. my desire is to eliminate that interface so usecase with direct usage of it are not that useful, I am searching for some higher level interfaces. Why org.openide.awt.HtmlRendererImpl is public? There are no tests neither for FilterNode, DataNode vs. HtmlStatus.
That code will not be enabled unless a line switch is set. It's for using font/color instead of * and [read only] in tab titles. A picture being worth a thousand words, it's far better for someone to be able to try it than for me to tell them about it. So what's the difference if I commit it now or later so people can try it? It will have no effect unless you run with a line switch (I may have done the diff while it was still set automatically to true because I was testing it - that's exactly what was the "more-or-less" I said when I attached it). HtmlRendererImpl should not be public. I'll change it. Use cases for HtmlRenderer.Renderer - why not read the diff for NodeRenderer? The point of it is: We need to supply a component which can render html, and it is typically used in tree/ table/list/combo boxes. All of these have slightly different one method interfaces in the JDK. HtmlRenderer.Renderer aggregates TreeCellRenderer, ListCellRenderer, TableCellRenderer, etc. into a single interface. But there are also some things you need to do sometimes that involve manipulating component properties of the renderer - the icon, the icon text gap, indentation. Two ways to solve this: 1. Have three methods on HtmlRenderer - say - asTableCellRenderer() asTreeCellRenderer() asListCellRenderer() - and document that the result is safe to cast as a JLabel if you want to manipulate it as one. That means - more work for the user of it - it must always *be* a JLabel, but this will not be enforced by the compiler - the entire API of JLabel is open for the caller to abuse on the shared renderer instance 2. Have HtmlRenderer.Renderer, and expose those things that are useful in a reasonable interface, and keep the flexibility to change how they are implemented or what concrete implementation is actually returned. HtmlLabelUI was not in the original plan; its purpose is that if any platform comes up with industrial strength fast html rendering in their UI, switching over to *not* use our renderer is as simple as using turning off use of our custom html rendering UI delegate and using the default. There will be tests for filter node before monday - that's another part of "more-or-less".
BTW, one other thing this patch enables - we will finally be able to "grey out" nodes that have been cut to the clipboard but not yet pasted (checking each node as it's painted to see if it's on the clipboard is not the way - possibly something using FeatureDescriptor.putValue() would work fast, if done carefully, maybe there's a better way). Still have to work out the mechanics of this, but it shouldn't be too hard. Ideas welcomed.
All good except: > But there are also some things you need to do sometimes that involve manipulating > component properties of the renderer - the icon, the icon text gap, indentation. Two ways > to solve this: > > 1. Have three methods on HtmlRenderer - say - asTableCellRenderer() > asTreeCellRenderer() asListCellRenderer() - and document that the result is safe to cast as > a JLabel if you want to manipulate it as one. That means > - more work for the user of it > - it must always *be* a JLabel, but this will not be enforced by the compiler > - the entire API of JLabel is open for the caller to abuse on the shared renderer instance > > 2. Have HtmlRenderer.Renderer, and expose those things that are useful in a reasonable > interface, and keep the flexibility to change how they are implemented or what concrete > implementation is actually returned. > 3. A lot of static methods, no type casting: public static TableCellRenderer asTableCellRenderer (); public static TableCellRenderer asTableCellRenderer (Icon icon); public static TableCellRenderer asTableCellRenderer (..., ..., ..., ..); I am affraid for having shared instance where one can call set methods which might suggest that the usage is to keep reference to the instance and call one setter on it, do something other, another setter, etc. Which might result in random bugs due to certain foreign code calling the setters meanwhile. While encourage the usage of the #3 would be: public TableCellRenderer getCellRenderer () { Icon icon = ...; String name = ...; return asTableCellRenderer (icon, name, ...): } that would imho prevent the probability of interference with foreign code.
"I am affraid for having shared instance where one can call set methods which might suggest that the usage is to keep reference to the instance and call one setter on it, do something other, another setter, etc. Which might result in random bugs due to certain foreign code calling the setters meanwhile. While encourage the usage of the #3 would be:" The HTML renderer is restricted to use on the AWT thread; so nobody can change its state mid-process without accessing it from another thread, which is easy to defend against. Basically, you take it, you paint with it, and you're done with it. One of the reasons for the small set of methods on Renderer is that any value set on them by a previous use will be reset to defaults on any call to getNNNCellRendererComponent, and the set of things that may be accessed are well defined, so it is easy to do this reset.
AWT thread restriction does not help. If you do: rend = sharedRenderer (); rend.setText (node.getName ()); rend.setIcon (node.getIcon ()); you are calling into foreing code and the node.getIcon() can in fact call sharedRenderer and reset the previous value of setText. This cannot happen in #3. Of course you can eliminate this in #2 by gathering the info first and then calling all the setters without using foreign code, but the API does not encourage that. That is why the only semantically correct way how to deal with calls to foreign code is: // gather all the info String name = node.getName(); String icon = node.getIcon(); // set it rend = sharedRenderer (); rend.setName (name); rend.setIcon (icon); // return it return rend; which is far more complex than return asTableCellRenderer (node.getName(), node.getIcon()); the semantically safe alternative of this in #3.
And how do you then set the icon-text-gap, possibly background color, etc. I'm afraid you end up with: asTreeCellRenderer (String text, Icon icon, Boolean isHtml, int margin, int iconTextGap, Color background, Color foreground, Border border...) which may be semantically safe, but not straightforward to work with, much less keep straight in one's head. The current API allows you to simply call getNNNCellRenderer, as defined in the JDK's API and use the result. The only time you need to intervene and set something is if: - The value object does not implement toString() to return the string you want to use - You want to use some different background color or something like that (by default it will use the tree's selection background color if selected, etc. - all the same stuff you get for free from DefaultTreeCellRenderer) - You want to adjust margin or icon text gap (we do both of these for things like ListView, which for whatever reason displays a tree-like indented view as a flat list) all of which is also true for DefaultNNNCellRenderer. I would much rather provide an easy-to-use API (by default you can just call, say, JTable.setDefaultRenderer (HtmlRenderer.sharedInstance()) and never think about it again unless you have any of the problems enumerated above). The odds of a Node (of all things) needing a table cell renderer in order to calculate its display name are so astronomical that the simple answer to anyone doing that is "What the hell are you thinking?!" Can you describe (I know you are imaginative!) some situation in which a Node would need to do that?
Note one interesting issue - tell me if this is a blocker or not. Once this is integrated, it will be impossible to run the constructor of any explorer view on any thread except the event thread. I can fix the test failures this causes for openide and core - no idea if there are other modules out there whose tests will do this. Now, IMO, (as well expressed elsewhere), running Swing constructors on any thread but the EQ is just a recipe for trouble. But finding all of the places in code that something might do this (I've fixed a few - mainly old actions that show a dialog with an explorer view, which don't override asynchronous() to return false as they should) may be hard. I could get rid of sharedRenderer(), and let people simply get an instance of HtmlRenderer.Renderer, and eliminate the handful of static vars that make the rendering code itself not thread-safe. Or we could treat it as welcome enforcement of reasonable practices. What do you think?
The method asTableCellRenderer with so many arguments is very similar to already existing HtmlRenderer.render (...) which takes 12. And as it is semantically correct I do not see reason why not use it. Imho the danger of two completely unrelated components spoiling its paint is so big that I would like to file that as tcr, but as I seem to be only one who cares, I'll leave it here as advice. If anybody else wants to support my opinion, file tcr otherwise keep it as minority opinion. I've been just talking to Petr Hrebejk and he proposed another way how to fix this: to have one Renderer per component. So instead of shared instance, have factory method and keep reference to the Renderer in the component. That would be solution to my concerns as well.
Note one option for the thread issue is simply to override getPreferredSize() to return some meaningless value if called on a non-EQ thread, in the various explorer view classes. The reason this is a problem is because BasicTreeUI calls instance methods on JTree when called from the superclass constructor, including getPreferredSize() - so whetever thread it's called on, getPreferredSize() will get called in the constructor, which in turn uses the cell renderer to determine its preferred size (as if it would know at that point).
"Imho the danger of two completely unrelated components spoiling its paint is so big" - the only way I can think of to do this is if you, say, embed one explorer view as a cell renderer of another which will first configure the renderer, and *then* paint the other explorer views, and then try to paint the renderer. Which is not how trees or tables in swing work. A compromise that I'd find acceptable: Keep the Renderer interface, but make it a factory interface instead. The sharedLabel() method will remain, as it is a straightforward way to do a few cell renderer like things, in particular, calculate the dimensions required by a string as rendered by the html renderer without needing it to be added to a parent to do so.
IMHO it is fine to limit explorer view construction to EQ if that is the most straightforward fix; should not be hard to check for (put in a Thread.dumpStack when it is violated and collect the bug reports).
The main problem is the fairly vast number of unit tests I'd need to fix by Monday...
Created attachment 14656 [details] Diff incorporating suggested changes - now a factory instead of using a shared renderer instance for everything
"The main problem is the fairly vast number of unit tests I'd need to fix" - to run in EQ? No problem, just add protected boolean runInEQ() {return true;} // override NbTestCase to each.
What kind of diff the last one is? I see just change in build.xml! I am not sure about Jesse's comment "limit explorer view construction to EQ", but it does not address my concerns at all. This Tim's comment is much better: > "Imho the danger of two completely unrelated components > spoiling its > paint is so big" - the only way I can think of to do this > is if you, say, embed one explorer > view as a cell renderer of another which will first configure > the renderer, and *then* paint > the other explorer views, and then try to paint the renderer. Which > is not how trees or > tables in swing work. Exactly! This is one case how things will get broken. But here another one which is more obvious: public String MyNode.getDisplayName () { return "myNode"; } public Image getIcon (...) { try { return computeIcon (...); } catch (IOException ex) { StatusDisplayer.getDefault().setStatusText ("<html>No <b>icon</b>"); return null; } } If the view is calling setText and then setIcon and the displayer is using setText and you keep the broken shared instance design this will render the node name as "No icon" and not "myNode".
Well, ignore the last diff, there will be another. Something I did in converting it to be a factory and misc small changes introduced a massive performance degredation.
I'm attaching a final (?) diff. Changes: - Went through the old comments, added correct @since tags, increment version of loaders and added apichange there, dealt with a couple other of Jesse's earlier comments that were still relevant - Html rendering is now thread safe, so explorer views can be constructed on other threads (this was true in the last diff as well) - Overrode getHtmlDisplayName() on a bunch of the filternodes displayed in the options tree for performance (no need to look up if they override getDisplayName()) - VisualizerNode now caches the html display name - Explorer tree now handles -fontsize() correctly again (this code seems to have just disappeared between 3.5.1 and present), as does TreeTableView - Corrected a rounding error in preferred size of tree cells that only becomes visible at around -fontsize 30 - Overrode addNotify() / removeNotify() in HtmlRendererImpl to do nothing (this was the performance problem's root) - HtmlRendererImpl will now parse out our custom UI manager syntax and return legal html if its UI is not an HtmlLabelUI, so running with Swing HTML painting really works (not that we want to do it) - A couple not directly relevant changes because of things I ran into while digging around for the performance problem (I know, I know, should be in a separate commit): - Fixed a blatant memory leak in TreeTableView (add a listener to the parent component in addNotify() and just leave it there) - Fixed the horizontal scrollbar problem in the options window - Fixed the background color of the property sheet description area (got mangled in the GTK merge)
Created attachment 14659 [details] final (?) dif
HtmlLabelUI is still public. createRenderer addresses my issues with various views sharing the same component. Fine. sharedLabel does not. Not fine from my point of view, but treat it as minority opinion. This comment goes thru whole your code. It may seem efficient, but it is dangerous. NodeRenderer shares static instances of renderer again, I really do not understand why allocating new instances makes such a big deal to compromise execution safety. Again minority opinion. Usually overideXYZ like overrideGetDisplayName are cached, at least I think. Otherwise fine, commit it so we can enjoy those little nice colored cvs statuses...
NodeRenderer was sharing static instances of the renderer before - I just changed ListView to be consistent with the rest of the universe; I don't really care, just as easy to change all the Explorer views to keep an instance of it. Re sharedLabel(), it's uses are extremely limited, but when it's useful it's useful. HtmlLabelUI should not be public, will fix. Re caching overridesGetDisplayName, maybe a good idea; but we are caching the result of getHtmlDisplayName() in VisualizerNode, which means it shouldn't be called more than once for the lifetiime of a visualizer node (well, unless the display name changes); that saves the call to FileSystem.HtmlStatus.annotateNameHtml() which would otherwise happen every on every paint as well; probably that's enough unless things other than explorer views start calling getHtmlDisplayName.
Merged. before merge tag: trunk_before_html_merge - core openide form projects vcscore after merge tag: trunuk_after_html_merge - core openide form projects vcscore
This issue was solved long time ago. Because nobody has reopened it neither added comments, we are verifying/closing it now. If you are still able to reproduce the problem, please reopen. Thanks in advance.