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 167063 - Toggle highlight search
Summary: Toggle highlight search
Status: RESOLVED FIXED
Alias: None
Product: editor
Classification: Unclassified
Component: Actions/Menu/Toolbar (show other bugs)
Version: 6.x
Hardware: PC Windows XP
: P4 blocker (vote)
Assignee: Vitezslav Stejskal
URL:
Keywords: NETFIX
Depends on:
Blocks:
 
Reported: 2009-06-14 16:31 UTC by olamagato
Modified: 2010-03-17 14:14 UTC (History)
2 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
Bad view of Toggle Highlight search (7.02 KB, image/png)
2009-06-14 16:32 UTC, olamagato
Details
proposed patch (10.41 KB, patch)
2010-03-08 17:48 UTC, dynamite
Details | Diff
second proposed patch (38.29 KB, patch)
2010-03-12 19:28 UTC, dynamite
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description olamagato 2009-06-14 16:31:04 UTC
Toggle highlight search button show bad status (always not pressed)
Comment 1 olamagato 2009-06-14 16:32:26 UTC
Created attachment 83558 [details]
Bad view of Toggle Highlight search
Comment 2 Jiri Prox 2009-06-15 16:43:59 UTC
What Look And Feel do you use?
Comment 3 olamagato 2009-06-16 11:19:52 UTC
I have classic (Windows95) L&F in Windows XP, Netbeans and all.
Comment 4 Jiri Prox 2009-08-19 14:21:28 UTC
The highlight result does not indicate by its appearance if it's pressed or not (tested on Ubuntu and Mac) 
Comment 5 olamagato 2009-08-19 16:13:29 UTC
Toggle button is always unpressed if mouse cursor is out of button rectangle and raised if button rectangle constains
mouse cursor. This button is never appears as pressed even than founded phrases are marked.

This bug continues its behavior on version 6.7.1 on classic Windows L&F.
Comment 6 Jiri Prox 2009-08-20 10:27:11 UTC
yes, that was exactly what I've tried to say, 
the sentence should be:
"The highlight result does not indicate ITS STATE by its appearance if it's pressed or not"
Comment 7 dynamite 2010-02-21 16:11:40 UTC
I'd like to NetFIX [1] this bug. Is it possible? [1] http://wiki.netbeans.org/NetFIX
Comment 8 Vitezslav Stejskal 2010-02-22 03:13:35 UTC
Yes, why not. Please make the toggle button also work consistently with the search bar's 'Highlight Results' checkbox (Ctrl+F). Thanks
Comment 9 Jiri Kovalsky 2010-02-22 03:41:40 UTC
Great, thanks Vito. Added NETFIX keyword then.
Comment 10 dynamite 2010-03-08 17:48:52 UTC
Created attachment 94881 [details]
proposed patch

The Editor's toolbar seems to be implemented through two mechanisms: through an annotation or alternatively directly using layer.xml.  An example of the later was the toggle bookmark button.  The toggle highlight though was modelled as an annotation which didn't seem to allow for visual toggling of the button.  ActionFactory did however contain inner classes similar to that used for the bookmark toggling button.  My solution was to remove the annotation and define a layer.xml file to define the buttons position in the toolbar.

The existing classes strongly suggests that the toggle highlight button did used to work and that its failure to do so in recent versions was a regression.
Comment 11 Jiri Kovalsky 2010-03-09 03:52:39 UTC
Vito, could you please review this patch and integrate it if you see no problems? Thanks a lot!
Comment 12 Vitezslav Stejskal 2010-03-09 03:59:06 UTC
Cool, thanks for the patch. I know this is going to sound weird, but could you please rearrange the stuff so that editor.lib does not depend on openide.text and openide.nodes? Thanks a lot
Comment 13 dynamite 2010-03-09 06:22:17 UTC
I half-suspected that would be the case.  I'll get on to it.
Comment 14 dynamite 2010-03-12 19:28:18 UTC
Created attachment 95127 [details]
second proposed patch

I have added extra EditorAction* annotations to enable registration of toggle buttons.  EditorActionButtonModel tags the ButtonModel that is associated with the action and EditorActionTextComponent which is attached to the optional method that is used to register the JTextComponent with that model.  EditorActionRegistrationProcessor adds corresponding extra properties into the generated layer which is then used by NbEditorToolbar to construct the toolbar buttons.

MyGaGaButton is now a sub-class of NbEditorToolbar and renamed GenericButton, although there may be a better name.

I have also taken the liberty of converting the bookmark toolbar actions to using the annotations, providing consistency.  This meant that the duplicated MyGaGaButton was no longer needed.  It also seemed that ToggleBookmarkAction$BookmarkButtonModel provided a more general test of the new annotations than the highlight functionality because it required access to the editor's text component.
Comment 15 Vitezslav Stejskal 2010-03-15 09:43:30 UTC
Thanks a lot for this. I'll have a look at the patch soon.
Comment 16 Vitezslav Stejskal 2010-03-15 13:43:50 UTC
http://hg.netbeans.org/jet-main/rev/9e29c40bc86a

I admit that editor actions are still mess, even though the situation has improved with the @annotations. I'm sorry, but I'm not going to apply the second patch either. I understand and appreciate your attempt to improve the situation and create a generic support for implementing editor toolbar toggle buttons, however, the way how it's implemented is not ideal. We definitely don't want the two new @annotations. dynamite, thanks a lot for your effort. I just hope that this will not come across as too harsh or arrogant.
Comment 17 dynamite 2010-03-15 14:35:22 UTC
That's okay.  It was my hesitation over the need to add/change the annotations that lead me to the first implementation.  If you have any thoughts as to how else it could be done then I could make a final attempt.  It feels wrong and inconsistent for the bookmark button to toggle and not the highlight button (particularly as the highlight button used to toggle).
Comment 18 dynamite 2010-03-15 14:38:10 UTC
I just noticed that this is now fixed.  Does this mean that you've implemented it a different way?  I so then thank you and I'll have a look at the real patch to see have I should have done it!
Comment 19 Vitezslav Stejskal 2010-03-15 15:46:21 UTC
(In reply to comment #18)
> I just noticed that this is now fixed.  Does this mean that you've implemented
> it a different way?

Yes, it's fixed in http://hg.netbeans.org/jet-main/rev/9e29c40bc86a. I simply stopped using @EditorActionRegistration for ToggleHighlightSearchAction and register it manually from BaseKit.createActions(). This way an instance of ToggleHighlightSearchAction is added to the editor component, rather than AlwaysEnabedAction generated by @EAR. And ToggleHighlightSearchAction implements Presenter.Toolbar and provides MyGaGaButton as the toolbar presenter.

In other words, 9e29c40bc86a just fixes the regression introduced somewhere along the way of converting editor actions to use @EAR, but it does not clean up the things.
Comment 20 Quality Engineering 2010-03-16 05:15:01 UTC
Integrated into 'main-golden', will be available in build *201003160201* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/9e29c40bc86a
User: Vita Stejskal <vstejskal@netbeans.org>
Log: #167063: fixing the editor toolbar toggle button for highlighting editor search results
Comment 21 dynamite 2010-03-16 08:22:22 UTC
The toggle button is defaulting to selected when no search has yet taken place.  I think the FIND_HIGHLIGHT_SEARCH property in EditorFindSupport.java needs to be set to false.
Comment 22 Vitezslav Stejskal 2010-03-17 14:14:28 UTC
I just checked the behavior in 6.5.1 and it is the same as now - ie. the button is in the selected state by default and the highlighting is turned on. Unless there is a strong need to change this I am in favor of keeping the current state.