Please use the Apache issue tracking system for new NetBeans issues (https://issues.apache.org/jira/projects/NETBEANS0/issues) !!
Bug 178751 - "Convert to composite component" is greyed out
"Convert to composite component" is greyed out
Status: VERIFIED FIXED
Product: javaee
Classification: Unclassified
Component: JSF
6.x
All All
: P3 (vote)
: 6.x
Assigned To: Marek Fukala
issues@javaee
68patch1-verified
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-12-16 12:16 UTC by edburns
Modified: 2010-02-11 13:22 UTC (History)
7 users (show)

See Also:
Issue Type: DEFECT
:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description edburns 2009-12-16 12:16:49 UTC
I created a simple JSF Project with facelets and added the following code into my index.xhtml page:

        <h:panelGrid columns="2">

            <h:outputText value="Userid:" /> <h:inputText value="#{model.userid}" />

            <h:outputText value="Password:"/> <h:inputSecret value="#{model.password}" />

            <h:commandButton binding="#{model.actionSource}" value="Login" />

        </h:panelGrid>

I highlighted the text and was about to click, "Convert to composite component" but I saw that it was greyed out!  

Oof, this is my favorite feature!  Can someone fix it fast?
Comment 1 Alexey Butenko 2009-12-17 02:36:22 UTC
There are basic check of the selection implemented, so it will
be grayed out if your selected text doesn't contain open and close tags.
Mark as incomplete.
Comment 2 David Konecny 2009-12-17 13:33:34 UTC
Alexey, why this was closed as incomplete? If I take given code and paste it into index.xhtml and select it then 'Convert to Composite Component' is disabled despite selection starting before opening tag and ending after closing tag. So the problem is reproducible and your explanation is not applicable for this case.
Comment 3 David Konecny 2009-12-17 14:49:47 UTC
I debugged this quickly and there seems to be at least two problems:

#1) if xhtml contains expression language then htmlResult.elementsList() returns tokens with offsets which do not correspond to real document offset. Marek knows more whether these offset needs to be translated to real ones or whether something else then htmlResult.elementsList() should be called instead or whether there is a bug somewhere else. To test this just use code snippet provided by Ed and dump tokens and you will see for example that instead of real tokens:

Element(tag)[591,647] "<h:inputSecret value="#{model.password}">" - "h:inputSecret" - {TagAttribute[name=value; value="#{model.password}"; nameOffset=622; valueOffset=628], }
Element(endtag)[648,663] "</h:inputSecret>" - "h:inputSecret" - {}

you will get

Element(tag)[591,633] "<h:inputSecret value="@@@">" - "h:inputSecret" - {TagAttribute[name=value; value="@@@"; nameOffset=622; valueOffset=628], }
Element(endtag)[634,649] "</h:inputSecret>" - "h:inputSecret" - {}

Consequently because of shifted offsets wrong tags are considered being "selected" and action is disabled.

#2) logic which enables/disables action does not take into account tags in format <tag/> - it always expect separate opening and closing tag

There is no workaround I'm aware of part from not using EL (in whole document) and always open and close tags.

Clearly this feature was not tested at all before release and would be good to include it in some test plans/automated tests.
Comment 4 Martin Schovanek 2009-12-18 04:55:27 UTC
Davide do we have a spec or description for this feature?
Comment 5 Marek Fukala 2009-12-18 06:29:16 UTC
After some discussion with the QE we agreed on following behaviour:
1) the Convert to Composite component popup menu will be removed
2) Selection sensitive code hint "Convert to Composite components" will be added
3) New insert code action "Inject Composite Component" will be added

The same template dialog will be opened for both action, in the first case there will be the selected code shown in the bottom textarea

4) for #1 there will be a warning in the dialog notifying the user that the selection contains not paired *JSF* tags (html tags matching doesn't matter), with a notice that the generated code may be invalid. However the wizard will allow to finish.

Following bugs also needs to be fixed:
1) hardcoded "ez" prefix - new input field will be added to the visual panel. Existing prefixes needs to be checked to avoid clashes.
2) missing second step name in the wizard
3) missing code coloring in the "Implementation section" textarea in the wizard
4) use of JSF "auto tag import" instead of fifty lines of proprietary code to insert the desired namespace declaration
Comment 6 Marek Fukala 2009-12-18 10:54:00 UTC
web-main#ab8ff28ea9b3

proposed functionality changes #1-4 has been done.

bug #4 fixed.

and of course the algorithm to find if the selection is valid or not has been reimplemented and is now embedding aware. What matters now is that there aren't any partially selected JSF tags, html content doesn't play any role.

I'm keeping the issue opened as P3 since bugs #1-3 are still valid.
Comment 7 Quality Engineering 2009-12-19 00:25:11 UTC
Integrated into 'main-golden', will be available in build *200912190200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/ab8ff28ea9b3
User: Marek Fukala <mfukala@netbeans.org>
Log: #178751 (partial fix) -  "Convert to composite component" is greyed out
Comment 8 David Konecny 2009-12-20 14:37:13 UTC
re. "Davide do we have a spec or description for this feature?" - if there is one then Marek or Alexey know about it.

re. "replacing Convert to Composite component action with editor hint" - I have not tested the new implementation and I do not know arguments based on which you've decided to implement it like this but I tend to disagree. Convert to Composite Component (C2CC) is not *hint*. Hint is an advice editor can give you to improve your code or alert you about commonly made mistakes etc. Just selecting some code should never give you hint "convert this to composite components" - that's not advice. In that sense I think that C2CC is action which user decides to perform similarly as most of the refactoring actions for example. Also something what initially was one single action - turn this selected code snippet into CC and replace it with CC call - is exactly what user wants to do and in that sense we are breaking it into two separate actions is step in wrong way instead of improvement of existing feature. Perhaps I'm just missing details of your discussion based on which you've decided for these changes but so far it looks like implementation difficulties/issues are driving IDE's UI and what user actually wants is being ignored.

re. "3) New insert code action Inject Composite Component" - 'inject' is sexy word which everybody seems to like to use these days, but I do not think it is accurate in this context. All what action does is that it *inserts* call of CC, right? If that's true then this Insert Code item should be called "Composite Component Call" or "Call Composite Component" or ... 

Could we also keep this as P2 and file other problems as separate P3 issue(s)? I would like to make sure it does not slip from Sustaining radar and gets fixed in a patch. It is not essential feature without users could not use IDE but composite components is new JSF 2.0 feature which NetBeans should propagate or advertise and right now it is completely broken and unusable.

Sorry if this response sounds overly negative. It is not how it is meant. Marek, QE, Alexey, ... you are doing great job addressing editor issues so promptly and me, IDE users, ... we are appreciating it! :-) Thanks.
Comment 9 Marek Fukala 2009-12-21 06:06:36 UTC
(In reply to comment #8)
> re. "Davide do we have a spec or description for this feature?" - if there is
> one then Marek or Alexey know about it.
I am not aware of any, maybe Alexey has one.

> 
> re. "replacing Convert to Composite component action with editor hint" - I have
I tend to agree with you David, at least from the point that the C2CC is kind of refactoring an as such should be consistent with the rest of the editors. OTOH the action "Convert to Composite Component" itself being in the popup looks at least weird if not stupid IMO. Sure we can introduce Refactoring -> C2CC action so it at least looks similar as the rest of refactorable editors. But my impression from this arrangement is that it is too flagrant that that we simply have no refactoring, which makes no good impression :-). So the hint was a kind of workaround for that. Once we do the html/xhtml/jsf refactorings (hopefully in 6.9) there will be "Refactoring" popup where the C2CC will logically be. 

> 
> re. "3) New insert code action Inject Composite Component" - 'inject' is sexy
> word which everybody seems to like to use these days, but I do not think it is
> accurate in this context. All what action does is that it *inserts* call of CC,
> right? 
Not exactly, it creates the composite component and adds a reference to it which is what most people considers as "inject" AFAIK. 

However if you look at the real implementation or source you'll realize there is just "Composite Component" in the Generate "insert code menu" which seems to be fine to me.

MSG_InjectCompositeComponentHint=Composite Component

Call to Composite Component is really ugly anyway :-) 

> Could we also keep this as P2 and file other problems as separate P3 issue(s)?
> I would like to make sure it does not slip from Sustaining radar and gets fixed
Yes, so then I consider this as fixed.

> in a patch. It is not essential feature without users could not use IDE but
> composite components is new JSF 2.0 feature which NetBeans should propagate or
> advertise and right now it is completely broken and unusable.
> 
> Sorry if this response sounds overly negative. It is not how it is meant.
> Marek, QE, Alexey, ... you are doing great job addressing editor issues so
> promptly and me, IDE users, ... we are appreciating it! :-) Thanks.
Not offended at all, I like your ideas. Thanks!
Comment 10 edburns 2009-12-21 08:42:23 UTC
MF> After some discussion with the QE we agreed on following behaviour:
MF> 1) the Convert to Composite component popup menu will be removed
MF> 2) Selection sensitive code hint "Convert to Composite components" will be
MF> added
MF> 3) New insert code action "Inject Composite Component" will be added

MF> OTOH the action "Convert to Composite Component" itself being in the
MF> popup looks at least weird if not stupid IMO.

[discussion between David and Marek deleted]

I suggest that the name be chosen to make it clear that some text in the
page will be replaced with the composite component.  The name "Inject
Composite Component" does not say that to me, but "Convert to Composite
Component" does.

I agree it is a refactoring and that it was weird to have it in the
popup.  The "advice" option is decent, but David had some reasons why it
was not quite right.  Perhaps the refactoring menu is the right place.

MF> 1) hardcoded "ez" prefix - new input field will be added to the visual panel.
MF> Existing prefixes needs to be checked to avoid clashes.
MF> 2) missing second step name in the wizard
MF> 3) missing code coloring in the "Implementation section" textarea in the wizard
MF> 4) use of JSF "auto tag import" instead of fifty lines of proprietary code to
MF> insert the desired namespace declaration

These are all good changes. One more is to use "cc" as the prefix in the
composite component page, rather than "composite".
Comment 11 David Konecny 2009-12-22 02:39:23 UTC
Thanks. I'm playing with latest dev build and here are some more comments:

"Selection sensitive code hint 'Convert to Composite components'" - the name of the hint in today's build is just "Composite components". It works well but I worry about discoverability of this feature. Hint icon in the editor gutter is the same icon for all hints which does not help either. The name is pretty vague too. I would rather have Refactoring menu with one action (named "Convert to Composite Component") than hiding this action from user. Some people might think it looks empty to have just one refactoring action but I would argue that it is better than no action. :-) 

"[Inject] Composite Component" - naming is OK. After CC is created and inserted I would expect to get CC source opened in editor and be ready for editing.

Do you think Marek that your fix (I have not looked at it) is simple enough to be backported to 68 patch?
Comment 12 Marek Fukala 2009-12-23 04:57:20 UTC
(In reply to comment #11)
> Thanks. I'm playing with latest dev build and here are some more comments:
> 
> "Selection sensitive code hint 'Convert to Composite components'" - the name of
> the hint in today's build is just "Composite components". 
Yes, this was a "typo". I've changed the hint's description to the same text as its fix has.

> worry about discoverability of this feature. Hint icon in the editor gutter is
> the same icon for all hints which does not help either. The name is pretty
> vague too.
Is it still so vague when changed to "Convert to Composite Component"?

> I would rather have Refactoring menu with one action (named "Convert
> to Composite Component") than hiding this action from user. Some people might
> think it looks empty to have just one refactoring action but I would argue that
> it is better than no action. :-) 
OK. I'll add Refactor -> Convert to Composite Component items.

> 
> "[Inject] Composite Component" - naming is OK. After CC is created and inserted
> I would expect to get CC source opened in editor and be ready for editing.
Yes, that might be better. I'll address it along with the other changes (bugs #1-3).

> 
> Do you think Marek that your fix (I have not looked at it) is simple enough to
> be backported to 68 patch?
I guess for 6.8 it would be better not to change the UI and just backport the fix of the algorith deciding whether the action should be enabled or disabled. What do you think? In such case, I'll do the backport myself. The issue is already marked as patch candidate.
Comment 13 Marek Fukala 2009-12-23 05:15:57 UTC
> MF> OTOH the action "Convert to Composite Component" itself being in the
> MF> popup looks at least weird if not stupid IMO.
> 
> [discussion between David and Marek deleted]
> 
> I suggest that the name be chosen to make it clear that some text in the
> page will be replaced with the composite component.  The name "Inject
> Composite Component" does not say that to me, but "Convert to Composite
> Component" does.
There isn't and has never been the word "Inject" used in the jsf sources. I used the term in the issue description, but then realized that the "insert code menu has already a title "Generate" so used just "Composite Component" description. So the discussion makes no sense. I'm sorry for the confusion.


> These are all good changes. One more is to use "cc" as the prefix in the
> composite component page, rather than "composite".
I've taken the original library prefixes from the TLDs AFAIR, but I can change it of course.
Comment 14 Marek Fukala 2010-01-04 10:00:30 UTC
I've fixed the wording, the default composite library prefix and added the refactoring action + the editor submenu. However I cannot push the change because of the Internal Server Error.

changeset:   156088:56ab6f0abb85
user:        Marek Fukala <mfukala@netbeans.org>
date:        Wed Dec 23 12:48:00 2009 +0100
summary:     C2CC hint has now the same description as its fix - conver to composite component

changeset:   156230:b037303ce61f
user:        Marek Fukala <mfukala@netbeans.org>
date:        Mon Jan 04 13:49:27 2010 +0100
summary:     using "cc" as default composite components library prefix

changeset:   156232:151b98f3abea
tag:         tip
user:        Marek Fukala <mfukala@netbeans.org>
date:        Mon Jan 04 17:58:30 2010 +0100
summary:     #178751 -  adding Refactoring action "Convert To Composite Component" for JSF xhtml files, also adding the Refactor submenu into editor popup
Comment 15 Marek Fukala 2010-01-06 05:01:19 UTC
changeset:   156233:8ef15fbd7dd7
tag:         tip
user:        Marek Fukala <mfukala@netbeans.org>
date:        Wed Jan 06 12:59:26 2010 +0100
summary:     #178751 - fixing wizard step name, fixing issue in the panel layout, replacing the plain preview by html editor's editor pane.
Comment 16 Marek Fukala 2010-01-06 05:16:30 UTC
changes finally pushed to web-main.

AFAIK the only undersolved issue now remains:

1) hardcoded "ez" prefix - new input field will be added to the visual panel.
Existing prefixes needs to be checked to avoid clashes.

Which I belive it P3.5 issue.

Can this one (P2) be closed then?
Comment 17 Marek Fukala 2010-01-06 06:38:40 UTC
Aha, I've just realized the issue is P3 only, closing then. Reopen if you have any objections to the solution. Thanks all for the feedback!
Comment 18 edburns 2010-01-06 09:31:55 UTC
I'll try this now.  I downloaded web-main build 2331 yesterday, but I guess this fix was not in it.
Comment 19 Quality Engineering 2010-01-06 23:39:31 UTC
Integrated into 'main-golden', will be available in build *201001070200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/151b98f3abea
User: Marek Fukala <mfukala@netbeans.org>
Log: #178751 -  adding Refactoring action "Convert To Composite Component" for JSF xhtml files, also adding the Refactor submenu into editor popup
Comment 20 Jindrich Sedek 2010-01-11 06:41:00 UTC
I tried to verify, but I faced an issue #179377 that should be fixed and integrated into patch with this issue as well
Comment 21 Marek Fukala 2010-01-12 11:12:44 UTC
I've finally fixed the last remaining problem with hardcoded prefix. There's a new input field where user can specify the library prefix, default valud is generated from the library's folder name. Checks for already used prefixes added.

fixed in web-main#03ac3a99c553

also note fix of issue #179379 -  (convert to composite component doesn't generate correct JSF namespaces), which is a kind of "enhancement" :-). A side effect of this fix is that unused namespaces are now not created in the created composite component's code.
Comment 22 Jindrich Sedek 2010-01-18 06:47:08 UTC
verified.
NetBeans IDE Dev (Build 201001180201)
Comment 23 edburns 2010-01-18 20:28:04 UTC
Hi,

I used to go to http://bertram.netbeans.org/hudson/job/web-main/ to get my latest builds but that doesn't seem to be up any more.  Where should I go now?
Comment 24 Petr Jiricka 2010-01-19 01:33:42 UTC
There are ongoing problems with the Bertram machine, you can use http://bits.netbeans.org/netbeans/trunk/nightly/ instead.
Comment 25 pgebauer 2010-01-19 05:34:05 UTC
The fix has been ported into the release68_fixes repository.
http://hg.netbeans.org/release68_fixes/rev/f23c127544e3

The fix has been ported together with the bugfix of issue #179379. If a
rollback is needed, both issues have to be skipped together.
Comment 26 Jindrich Sedek 2010-01-27 04:57:06 UTC
verified in 68_fixes build
NetBeans IDE 6.8 (Build 201001261800)
Comment 27 edburns 2010-02-11 13:22:23 UTC
Ok, I see this working now.  Thank you very much.


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2014, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo