FWD » Core Development » User Interface

General Attributes

• COLUMN-FGCOLOR
• MIN-HEIGHT-CHARS
• MANUAL-HIGHLIGHT

Editor Methods

• EDIT-CUT
• EDIT-COPY
• EDIT-PASTE

Browse Attributes

• PREV-COLUMN
• NUM-VISIBLE-COLUMNS

Browse Options

• DISABLE-AUTO-ZAP
• VIEW-AS COMBO-BOX is NOT being done here, see #3908

Browse Methods

• ADD-COLUMNS-FROM

The browse cell support for combo-box is discussed (briefly) in #2628-5.

From Constantin:

the browse COMBO-BOX column might be a little tricky to add

Since combo-box has a child window for the drop-down, I can see your point. On a positive note, Stanislav already added support for toggle-box so that means the core support for non-fill-in cells is already there.

What is the branch to commit the changes for this task?

Create a new one.

What is the current status, can someone clarify? I see the support is implemented for Browse Column widget, at least on conversion level. What is TODO for this attribute?

Stanislav: this one is for you, I think.

Another question for browse widget. Why we currently have the COLUMN-FGCOLOR attached to BrowseColumnWidget instead of BrowseWidget? It it planned approach? I'm asking because BrowseColumnWidget is an artificial one, no direct counterpart in 4GL code, right? Using this attribute is something like browse:COLUMN-FGCOLOR, not browse-column:COLUMN-FGCOLOR. How it should work from 4GL code conversion perspective?

Created task branch 3811a from trunk revision 11301.

What is the current status, can someone clarify? I see the support is implemented for Browse Column widget, at least on

COLUMN-FGCOLOR attribute is fully implemented. Other items are not.

Stanislav Lomany wrote:

What is the current status, can someone clarify? I see the support is implemented for Browse Column widget, at least on

COLUMN-FGCOLOR attribute is fully implemented. Other items are not.

How about #3811-10 clarification? This will significantly save my time from writing additional testcases and check the browse widget.

I'm asking because BrowseColumnWidget is an artificial one, no direct counterpart in 4GL code, right?

I would say it has direct counterpart. You can get columns by using browse:FIRST-COLUMN and column:NEXT-COLUMN attributes. Here you can find the full list of column widget attributes/methods: https://documentation.progress.com/output/ua/OpenEdge_latest/index.html#page/dvref/browse-widget.html

The list of attributes/methods has the "Applies to" column which tells if the attributes/method is applicable to a browse, a column or a cell, e.g. BGCOLOR attribute is applicable to all three elements so we have browse:BGCOLOR attribute and column:BGCOLOR attribute. Cell, on the other hand, is not a widget and its attributes are accessed in the ROW-DISPLAY trigger through the handle to the column to which the cell belongs. E.g.

column1 = browse brws:first-column. 

on row-display of browse brws do:
  column1:bgcolor = random(6, 13).
end.

Cell methods like EDIT-COPY can be accessed for editable browse when editing is in process using the same column handle: column1:edit-copy().

Using this attribute is something like browse:COLUMN-FGCOLOR, not browse-column:COLUMN-FGCOLOR.

E.g. browse:FIRST-COLUMN:COLUMN-FGCOLOR.

Stanislav Lomany wrote:

...

Using this attribute is something like browse:COLUMN-FGCOLOR, not browse-column:COLUMN-FGCOLOR.

E.g. browse:FIRST-COLUMN:COLUMN-FGCOLOR.

OK. Thanks for clarification. Shouldn't the browse:COLUMN-FGCOLOR set or get the column color for all columns in a browse?

Task branch 3811a has been updated for review to revision 11302.

The update adds conversion support for browse widget attributes (MIN|MAX)-HEIGHT-CHARS. Also adds conversion support for MAX-WIDTH_CHARS window widget setter. The runtime support for browse widget server side has also been added. Starting to implement client side runtime support for browse widget.

Do we need to implement MAX-HEIGHT-CHARS attribute in addition to MIN-HEIGHT-CHARS?

Do we need to implement MAX-HEIGHT-CHARS attribute in addition to MIN-HEIGHT-CHARS?

Yes, that makes sense.

OK. Thanks for clarification. Shouldn't the browse:COLUMN-FGCOLOR set or get the column color for all columns in a browse?

In 4G there is no such attribute:

COLUMN-FGCOLOR attribute          Column

You can use FGCOLOR for that.

Task branch 3811a has been updated for review to revision 11303.

This finalizes the implementation of the (MIN|MAX)-HEIGHT-CHARS attribute. After testing I've found the actual worker for browse widget is MIN-HEIGHT-CHARS and only for reading. The other usages of the attribute produces the following error/message box:

**MAX-HEIGHT is not a queryable attribute for BROWSE br(4052).
**MAX-HEIGHT is not a setable attribute for BROWSE br(4052).
**MIN-HEIGHT is not a setable attribute for BROWSE br(4052).

So the maxHeightChars variable was removed from BrowseConfig. We do not need to server-client exchange here. However the 4GL does allow to use the attributes for set and get. So the code should be converted without errors, the rules allow the setters to be converted. The error message is displaying at run time.

So far the (MIN|MAX)-HEIGHT-CHARS is now completed and verified to have the same reactions as 4GL. For browse and window widgets. Shifting to another attribute implementation.

Task branch 3811a has been updated for review to revision 11304.

This adds conversion and runtime support for browse attributes NUM-VISIBLE-COLUMNS and PREV-COLUMN. Shifting to the other methods/options.

Task branch 3811a has been updated for review to revision 11305.

This adds support for DISABLE-AUTO-ZAP attribute in browse widget(column). Currently the implementation works only for fill-in based cell value. Planning to check the attribute used as option in DEFINE BROWSE ... statement.

Task branch 3811a has been updated for review to revision 11306.

The update adds support for DISABLE-AUTO-ZAP option in DEFINE BROWSE statement. The idea is to use currently implemented use of setAutoZap() attribute on the frame set up stage. The default value for DISABLE-AUTO-ZAP is changed to false. Going to start working on remaining Editor/Browse widgets methods.

Greg,

I've started to work on clipboard related editor widget methods: CUT/COPY/PASTE and I have two design questions to clarify.

1. The Clipboard we will use here is the client side object, correct? For web client this is the system clipboard of the machine where the Web browser is running, correct? This way we can provide text exchange between FWD application and other text edit capable utils(like Notepad for example).

2. I'm going to add new methods to existed SelectableText interface instead of creating new special interface for clipboard methods. This integrates the clipboard usage features not only to Editor widget but Fill-In(and all classes extended from) will also be upgraded. Is it OK? Or we need only Editor be involved in this change?

The widgets are currently involved in point 2 changes - FillInWidget and ComboBoxWidget in addition to EditorWidget. We can just add a stub code if particular implementation is not required at this time for fill-in and combo-box.

Task branch 3811a has been updated for review to revision 11303.

Looks good.

Task branch 3811a has been updated for review to revision 11304.

Look good.

Task branch 3811a has been updated for review to revision 11305-6.

Looks good.

1. The Clipboard we will use here is the client side object, correct?

Yes. We have existing support for integrating with the clipboard. That should be used here.

For web client this is the system clipboard of the machine where the Web browser is running, correct? This way we can provide text exchange between FWD application and other text edit capable utils(like Notepad for example).

Yes. But the low level clipboard integration is already there so you don't have to write that part.

2. I'm going to add new methods to existed SelectableText interface instead of creating new special interface for clipboard methods. This integrates the clipboard usage features not only to Editor widget but Fill-In(and all classes extended from) will also be upgraded. Is it OK?

Yes, that is great.

We can just add a stub code if particular implementation is not required at this time for fill-in and combo-box.

No, please go ahead with a full implementation for all text widgets.

Also, please note that EDIT-UNDO was probably only stubbed in the POC (see #3762). But we need the runtime for it, so please add EDIT-UNDO to the list. If it is easy to implement EDIT-CAN-UNDO and EDIT-CAN-PASTE, then go ahead with those.

On UNDO, FILL-IN is not listed but it is supported as part of a BROWSE cell. I suspect it works in a regular FILL-IN too, but you should check.

Greg Shah wrote:

Also, please note that EDIT-UNDO was probably only stubbed in the POC (see #3762). But we need the runtime for it, so please add EDIT-UNDO to the list.

OK.

If it is easy to implement EDIT-CAN-UNDO and EDIT-CAN-PASTE, then go ahead with those.

OK. I'll check.

On UNDO, FILL-IN is not listed but it is supported as part of a BROWSE cell. I suspect it works in a regular FILL-IN too, but you should check.

OK.

Don't forget to update the gap analysis rules. By the way, EDIT-CAN-UNDO is marked wrong today (the conversion is marked none but it should be full).

Additional attributes/methods to implement:

• EDIT-UNDO

• AUTO-COMPLETION

Greg Shah wrote:

Don't forget to update the gap analysis rules.

OK.

By the way, EDIT-CAN-UNDO is marked wrong today (the conversion is marked none but it should be full).

May be you mean EDIT-UNDO instead? I'm asking because I have not found any references for kw_edit_c_u for EDIT-CAN-UNDO inside methods_attributes.rules. Or the support for this attribute defined in other rule file?

Task branch 3811a has been updated for review to revision 11307.

Adding full support for EDIT-(COPY|CUT|PASTE) methods. Testcases repo updated to rev 1824 to check editor/fill-in widgets with new methods.

I have found the runtime implementation has a bugs for Fill-In with handling selected text. Adding debug here in TODO list for this task. The Web client has some strange feature for CUT/COPY/PASTE. Only text copied inside FWD Web client can be pasted. Not text from system clipboard. I'm going to investigate this too in deep.

May be you mean EDIT-UNDO instead?

Yes, sorry.

Actually EDIT-CAN-UNDO implementation is easy if UNDO capability already done(as for EditorGuiImpl). So it is not an issue for editor to add EDIT-CAN-UNDO.

Another case is FillInGuiImpl. It is not UNDO capable as far as I can see. So here we need to add UNDO capability first if required, and then we can easily add EDIT-CAN-UNDO feature.

Task branch 3811a has been updated for review to revision 11308.

Adding support for EDIT-UNDO method, EDIT-CAN-UNDO, EDIT-CAN-PASTE attributes.

The investigation shows the UNDO operation is possible for Editor, the Fill-In is not undo aware. So far we have full support for these attributes and method. I have added EDIT-CAN-PASTE support. The idea is to try getting text from clipboard and if it is not empty we can consider the EDIT-CAN-PASTE attribute is true.

I need at least one day more to completely test how new attributes are working in real client environment especially for Web client. This must be done in addition to testing CUT/COPY/PASTE. And make fixes if issues will be discovered.

Status update from Eugenie:

Yes, almost completed. I have left to add implementation for Browse related ADD-COLUMNS-FROM method and attribute MANUAL-HIGHLIGHT.

Also I would like to spend a day to test/fix already implemented editor related features - CUT/COPY/PASTE/UNDO if you do not mind. I have a feeling the FWD has an issues here especially in Web client and Clipboard used in this case.

So totally I'm planning to spend up to 3 days for all(1 day for Clipboard related features testing and 1-2 days fro remaining new features implementation and tests). If something will go unexpected it can take more time(or may be less if implementation will be simple).

I'm good with the plan, except for MANUAL-HIGHLIGHT.

MANUAL-HIGHLIGHT is strange because it is not clear in which situations we need to honor this. In past applications, there was no difference when this was ON or OFF. But the feature must have some implementation, we just don't know which widgets are affected and what those effects will be. I was planning to ask a 4GL developer to write testcases so that we can know what to implement.

For now, leave MANUAL-HIGHLIGHT unfinished.

Greg Shah wrote:

I'm good with the plan, except for MANUAL-HIGHLIGHT.

MANUAL-HIGHLIGHT is strange because it is not clear in which situations we need to honor this. In past applications, there was no difference when this was ON or OFF. But the feature must have some implementation, we just don't know which widgets are affected and what those effects will be. I was planning to ask a 4GL developer to write testcases so that we can know what to implement.

For now, leave MANUAL-HIGHLIGHT unfinished.

OK. Understood. I thought the attribute is related to direct manipulation features. When we select some widgets and move or resize them by mouse. But certainly it is not clear how to substitute default highlight painter with custom one so let's wait for good test to see what exactly we need to do here.

One more attribute to implement I've missed: AUTO-COMPLETION for combo-box widget. We need this to be implemented, right?

Task branch 3811a has been updated for review to revision 11309.

This adds conversion support and runtime stubs for browse widget ADD-COLUMNS-FROM method.

Eugenie Lyzenko wrote:

One more attribute to implement I've missed: AUTO-COMPLETION for combo-box widget. We need this to be implemented, right?

Yes.

UNDO editor feature does not properly work in FWD. The text inside the editor is not getting back to the version before last update. Need to investigate:
1. The expected behavior in 4GL.
2. What is wrong in FWD.

UNDO 4GL actions:
1. Text deleted by Del or Backspace in current line - restore text making it as selected (Swing OK)
2. Cut text - restore text, making it as selected
3. Paste text - remove text, no selection
4. New line by ENTER - remove new line, no selection

The FWD experience/issues:
1. Swing and Web clients OK.
2. Swing and Web clients not working as expected, removed text restores in wrong position, selection is incorrect
3. Swing and Web clients not working as expected, no pasted text removing, position is wrong
4. Swing and Web clients not working

Web client specific. It is not possible to paste text from external application like gedit. Web client does paste from internal buffer only if text was previously copied by Web client application COPY/CUT operation. This is an issue/bug to be fixed, correct?

Eugenie Lyzenko wrote:

Web client specific. It is not possible to paste text from external application like gedit. Web client does paste from internal buffer only if text was previously copied by Web client application COPY/CUT operation. This is an issue/bug to be fixed, correct?

PASTE works in Web client AFAIK; but there is another problem - first key is 'consumed' by the Web JS driver - I don't think it reaches the Java client. How to duplicate: switch focus from browser to another window (like gedit), and back; type a key - first time will fail, second time will appear.

Constantin Asofiei wrote:

Eugenie Lyzenko wrote:

Web client specific. It is not possible to paste text from external application like gedit. Web client does paste from internal buffer only if text was previously copied by Web client application COPY/CUT operation. This is an issue/bug to be fixed, correct?

PASTE works in Web client AFAIK; but there is another problem - first key is 'consumed' by the Web JS driver - I don't think it reaches the Java client. How to duplicate: switch focus from browser to another window (like gedit), and back; type a key - first time will fail, second time will appear.

Are you able to PASTE from external application like gedit int FWD editor widget?

Eugenie Lyzenko wrote:

Are you able to PASTE from external application like gedit int FWD editor widget?

Yes. But first type any other key (which is consumed by JS driver) and after that CTRL-V.

Please try to fix this.

Constantin Asofiei wrote:

Eugenie Lyzenko wrote:

Are you able to PASTE from external application like gedit int FWD editor widget?

Yes. But first type any other key (which is consumed by JS driver) and after that CTRL-V.

Please try to fix this.

There is an issue with EDIT-PASTE() implementation(will be fixed as part of the EDIT-PASTE implementation). But I can paste by both CTRL-V and SHIFT-INSERT without any extra key. Can you provide detailed instruction to replicate the issue you have mentioned.

Eugenie Lyzenko wrote:

There is an issue with EDIT-PASTE() implementation(will be fixed as part of the EDIT-PASTE implementation). But I can paste by both CTRL-V and SHIFT-INSERT without any extra key. Can you provide detailed instruction to replicate the issue you have mentioned.

• focus an editable widget in the Web client (FILL-IN)
• switch focus to from Chrome to another OS window (like gedit) - use ALT-TAB for this
• switch focus back to Chrome - type any letter/number/etc, it will not be visible in the FILL-IN; second time you will see it. Don't press the mouse buttons or anything else - just have focus back to Chrome and you will see that the first time you type a key is ignored.

Constantin Asofiei wrote:

Eugenie Lyzenko wrote:

There is an issue with EDIT-PASTE() implementation(will be fixed as part of the EDIT-PASTE implementation). But I can paste by both CTRL-V and SHIFT-INSERT without any extra key. Can you provide detailed instruction to replicate the issue you have mentioned.

So, using chrome, I'm doing this:

• focus an editable widget in the Web client (FILL-IN)
• switch focus to from Chrome to another OS window (like gedit) - use ALT-TAB for this
• switch focus back to Chrome - type any letter/number/etc, it will not be visible in the FILL-IN; second time you will see it. Don't press the mouse buttons or anything else - just have focus back to Chrome and you will see that the first time you type a key is ignored.

What about Editor widget, not Fill-In? Do you see the issue in Editor? If not - it is another problem, not related to the EDIT-(CUT/COPY/PASTE) feature.

Eugenie Lyzenko wrote:

What about Editor widget, not Fill-In? Do you see the issue in Editor? If not - it is another problem, not related to the EDIT-(CUT/COPY/PASTE) feature.

Yes, what I describe (first key being consumed after Chrome receives focus) is another problem; and CTR-V works for both FILL-IN and EDITOR, with clipboard data from i.e. gedit.

Is your problem that EDIT-PASTE is not working if the clipboard has data from another OS window?

Constantin Asofiei wrote:

Eugenie Lyzenko wrote:

What about Editor widget, not Fill-In? Do you see the issue in Editor? If not - it is another problem, not related to the EDIT-(CUT/COPY/PASTE) feature.

Yes, what I describe (first key being consumed after Chrome receives focus) is another problem; and CTR-V works for both FILL-IN and EDITOR, with clipboard data from i.e. gedit.

Is the same issue for Firefox?

Is your problem that EDIT-PASTE is not working if the clipboard has data from another OS window?

No the problem is the Websock gets empty clipboard content when clipboard has the text. Until pressing paste shortcut inside Web client FWD editable widget. The CTRL-V forces the web client to return valid clipboard content instead of empty text. May be this is the Browser constraint and Web browser has it's own internal clipboard butter that become in sync with system clipboard by PASTE keys. For security reasons for example to protect from viruses and robots.

Task branch 3811a has been updated for review to revision 11310.

This is the set of fixes for EDITOR widget in UNDO operation processing. Including correct processing for new line insert(RETURN key), CUT and PASTE.

What is not yet fixed is PASTE operation from external application to Web client based FWD EDITOR widget. Looks like before any keyboard based operation inside browser it is problematic to access the clipboard content form JS code. Need further investigation for what we can do here.

Eugenie Lyzenko wrote:

Is the same issue for Firefox?

The issue I mentioned with the first key being lost after the browser gets back the OS focus, is only for Chrome; I can't duplicate it with Firefox.

Task branch 3811a has been updated for review to revision 11311.

Added runtime support for ADD-COLUMNS-FROM method. The gap rule file has also being updated to reflect current support level change.

Starting to implement AUTO-COMPLETION attribute.

Task branch 3811a has been updated for review to revision 11312.

Added first stage of the AUTO-COMPLETION attribute support. At this time full server side runtime implementation has been added including VIEW-AS clause. The conversion support already was ready to use. The remaining part is to implement GUI client side runtime implementation. And it is a bit complex than I have expected. The two testcases was created to investigate details of attribute usage in native 4GL(updated in repo as rev 1827).

What I've found is the attribute is active for SIMPLE and DROP-DOWN modes of combo-box widget, no matter if the list is opened or not. The item selection is performed based on string part entered in entry field. The respective item is selecting, making remaining item part as selected text inside entry field. In addition the VALUE-CHANGED trigger is firing with full item name to be set as combo-box new screen value. This is what we have to implement for client side part. Continue working.

What is not yet fixed is PASTE operation from external application to Web client based FWD EDITOR widget. Looks like before any keyboard based operation inside browser it is problematic to access the clipboard content form JS code. Need further investigation for what we can do here.

Sergey: can you discuss with Eugenie about the technique we use for providing a real but hidden entry field for clipboard processing in a fill-in. I wonder if we miss that in an editor.

Greg Shah wrote:

What is not yet fixed is PASTE operation from external application to Web client based FWD EDITOR widget. Looks like before any keyboard based operation inside browser it is problematic to access the clipboard content form JS code. Need further investigation for what we can do here.

Sergey: can you discuss with Eugenie about the technique we use for providing a real but hidden entry field for clipboard processing in a fill-in. I wonder if we miss that in an editor.

Don't understand what is the issue. PASTE should work with the web client, it was implemented and worked properly.

From this thread it is not clear what is the PASTE issue? Should I check out 3811a?

Eugenie, I'm confused, too - PASTE is working via CTRL-V with both Chrome and Firefox. Please post a standalone testcase and a scenario (browser, key pressed, which widget has focus, etc), which duplicates your issue.

Constantin Asofiei wrote:

Eugenie, I'm confused, too - PASTE is working via CTRL-V with both Chrome and Firefox. Please post a standalone testcase and a scenario (browser, key pressed, which widget has focus, etc), which duplicates your issue.

I have updated testcases bzr repo(rev 1828). There you can find the testcases uast/gui/edit_undo.p. Run it and follow the scenario:
0. Have 3811a branch.
1. Open external application(gedit for example)
2. Select some text and copy to system clipboard.
3. Go to the FWD application. Focus to the EDITOR widget.
4. Press PASTE pushbutton(do not use CTRL-V or SHIFT-INS, it is important).
5. You will see the Web client (unlike Swing one does not paste the text because JS gets empty clipboard content)

This is an issue we need to resolve(may this is planned Browser restriction or missing correct Clipboard API in Browser).

Eugenie Lyzenko wrote:

Constantin Asofiei wrote:

Eugenie, I'm confused, too - PASTE is working via CTRL-V with both Chrome and Firefox. Please post a standalone testcase and a scenario (browser, key pressed, which widget has focus, etc), which duplicates your issue.

I have updated testcases bzr repo(rev 1828). There you can find the testcases uast/gui/edit_undo.p. Run it and follow the scenario:
0. Have 3811a branch.
1. Open external application(gedit for example)
2. Select some text and copy to system clipboard.
3. Go to the FWD application. Focus to the EDITOR widget.
4. Press PASTE pushbutton(do not use CTRL-V or SHIFT-INS, it is important).
5. You will see the Web client (unlike Swing one does not paste the text because JS gets empty clipboard content)

This is an issue we need to resolve(may this is planned Browser restriction or missing correct Clipboard API in Browser).

I downloaded uast/gui/edit_undo.p. It seems that it can be implemented similar to the copy, paste, cut editor's menu popup (opened by the right mouse button).

This EditorPopupGuiImpl should work properly.

Eugenie Lyzenko wrote:

Guys, we have some misunderstanding here. Again, Swing client works fine, while Web client does not.

The approach with editor pop up menu does not work as well, see the picture, it shows there is nothing to paste.

I just tested the trunc version and found that the Paste is disabled too. But if the Paste menu is enabled, then the data can be transmitted from the system clipboard to the application and back. This is the issue of

   private void initEditorItems(boolean textEmpty)
   {
      cutOpt.setEnabled(!textEmpty);
      copyOpt.setEnabled(!textEmpty);

      String clipboardContents = ((GuiDriver) OutputManager.getDriver()).clipboardContents();
      pasteOpt.setEnabled(clipboardContents != null && !clipboardContents.isEmpty());

      selectAllOpt.setEnabled(textEmpty);
   }

OutputManager.getDriver()).clipboardContents() cannot be done via jscript without user's approval.

If we send CTRL+V like this

            Widget<?> owner = (Widget<?>) 
                     menu.screen().getRegistry().getComponent(menu.config().ownerId);

            EventManager.postOSEvent(new KeyInput(owner, EventType.KEY_PRESSED, Keyboard.KA_PASTE));

Sergey Ivanovskiy wrote:

Eugenie Lyzenko wrote:

Guys, we have some misunderstanding here. Again, Swing client works fine, while Web client does not.

The approach with editor pop up menu does not work as well, see the picture, it shows there is nothing to paste.

I just tested the trunc version and found that the Paste is disabled too. But if the Paste menu is enabled, then the data can be transmitted from the system clipboard to the application and back.

Yes, I know. The problem is not the Paste is disabled. The problem is the Web client(JS code) does not see the clipboard content. That's why the Paste is disabled. And this is the main issue here.

This is the issue of
[...]
This method ((GuiDriver) OutputManager.getDriver()).clipboardContents() doesn't return the system clipboard content for the web client because the process to get this value from the browser based on the user action. It cannot be done via jscript without user's approval.

This is what I'm struggling with.

It seems that this the Paste button should be enabled at the start, then you can observe that the PASTE works properly. I don't know if we can fix OutputManager.getDriver()).clipboardContents() in order to ask the user to get the clipboard.

Just enabling Paste button does not help because the browser will not see the Clipboard content whether we enable Paste or not.

For now the only potential solution I see is to simulate paste key sequence(say CTRL-V) on the JS level as if the user presses the real keyboard. But this is big Browser security hole and I think the Browser will struggle with our attempts like this.

Or may be we need some third party browser plugin that allows to easily get access to system clipboard from Browser.

I don't know the PASTE menu enabled requirements, but it seems that this menu item can be enabled forever.

Eugenie, I tested this issue and found that CTRL-V should work. It seems that your conclusions are incorrect. Did you test it with another branch with the trunc version? Please enable PASTE menu and test this issue.

Sergey Ivanovskiy wrote:

Eugenie, I tested this issue and found that CTRL-V should work. It seems that your conclusions are incorrect. Did you test it with another branch with the trunc version? Please enable PASTE menu and test this issue.

I mean that pasteOpt.setEnabled(true); is enough. Please apply this diff

=== modified file 'src/com/goldencode/p2j/ui/client/gui/EditorPopupGuiImpl.java'
--- src/com/goldencode/p2j/ui/client/gui/EditorPopupGuiImpl.java    2018-08-02 16:22:01 +0000
+++ src/com/goldencode/p2j/ui/client/gui/EditorPopupGuiImpl.java    2019-04-02 17:06:39 +0000
@@ -242,8 +242,7 @@
       cutOpt.setEnabled(!textEmpty);
       copyOpt.setEnabled(!textEmpty);

-      String clipboardContents = ((GuiDriver) OutputManager.getDriver()).clipboardContents();
-      pasteOpt.setEnabled(clipboardContents != null && !clipboardContents.isEmpty());
+      pasteOpt.setEnabled(true);

       selectAllOpt.setEnabled(textEmpty);
    }

There is a picture of the current COPY/PAST/CUT web client architecture in the original task where this issue is originated but I cannot find this task. Any case ((GuiDriver) OutputManager.getDriver()).clipboardContents(); usage for the web client and for the swing client are different due to the browser's issue.

Sergey Ivanovskiy wrote:

Sergey Ivanovskiy wrote:

Eugenie, I tested this issue and found that CTRL-V should work. It seems that your conclusions are incorrect. Did you test it with another branch with the trunc version? Please enable PASTE menu and test this issue.

I mean that pasteOpt.setEnabled(true); is enough. Please apply this diff
[...]
and test it with the trunc version to observe that it works properly.

Yes, this works properly requesting CTRL-V from user to confirm in a Browser window.

Eugenie, please look at SystemAction in p2j.mouse.js in order to understand how PASTE is implemented.

It seems that this code

      MouseHandler.registerWidgetAction(
               menuId,
               new SystemAction(menuId, MouseEvt.PASTE, null));

Thank you for help. I have some background to implement solution.

The difference is I do not need event listener. Instead FWD needs one time call to JS to initiate paste from Java code. Working on the implementation.

It is difficult to use this system menu for

on choose of PasteButton in frame ClipFrame
do:
  chEditor:edit-paste().
end.

Task branch 3811a has been updated for review to revision 11313.

This is complete implementation of the soft PASTE operation for EDIT-PASTE() widget method called from editor widget. Working in Web client as well. The pop up menu is not used here but idea is pretty similar. We have bi-directional relationship between Java and JS parts to sync the moment when data is ready for paste.

Because the code in EditorPopupGuiImpl is not used here I leave this file untouched, so the change mentioned in #3811-71 is not here.

Continue working with remaining AUTO-COMPLETION attribute TODO list.

The status updated, but I have not received the respective message to e-mail. Pushing another record.

Task branch 3811a has been updated for review to revision 11314.

The update adds base implementation for AUTO-COMPLETION runtime support. The SIMPLE mode was tested and confirmed as working as expected. The DROP-DOWN mode testing is now starting. The testcases used can be found in uast/combo_box, combo_box22.p and combo_box22_1.p for SIMPLE and DROP-DOWN modes respectively. I'm expecting to have some issues(deviations from 4GL) for the cases the drop-down list is opened. Need to examine in details and fix problems discovered.

I need to write the testcase to verify the current implementation of the ADD-COLUMNS-FROM method. I'm worry a bit and need to make sure my implementation is consistent.

Added testcase uast/browse/brws_add_cols_from.p to bzr testcases repo, rev 1829.

Found several bugs/issues in current implementation. Fixing.

The bugs found have been fixed. Now ADD-COLUMNS-FROM() works almost the same as in original 4GL.

The only deviation we have is related to Progress Document(ABL Reference) inconsistency. According to doc related to adding columns behavior(page 1412) I see:

...
If a field is found that already has a corresponding browse column created, it is ignored.
...

But testcase shows the different reaction in real 4GL environment. Adding is ignored if and only if the field was already added by previous ADD-COLUMNS-FROM(). If the field existed before first call to ADD-COLUMNS-FROM() it will really be added making fields duplication in browse.

If my understanding correct in cases when Progress Docs conflicts with 4GL reality we implement what we see on 4GL system, right? No matter how strange the reality is(I think the document describes more consistent behavior).

So I'm planning to add respective handling code if nobody objects.

Task branch 3811a has been updated for review to revision 11317.

Adding gap status change for AUTO-COMPLETION attribute.

If my understanding correct in cases when Progress Docs conflicts with 4GL reality we implement what we see on 4GL system, right? No matter how strange the reality is(I think the document describes more consistent behavior).

Yes.

Greg Shah wrote:

If my understanding correct in cases when Progress Docs conflicts with 4GL reality we implement what we see on 4GL system, right? No matter how strange the reality is(I think the document describes more consistent behavior).

Yes.

OK. The current version 11317 does match this criteria.

1. queryToCheck parameter is not needed in getFieldsForBuffer and getBufferForTable.
2. Instead of
   

   Buffer buffer = null;
   ...
   if (tableNameStr.isEmpty())
   {
      return buffer;
   }
   ...
   
   boolean retCode = false;
   if (srcBuffer == null)
   {
      return new logical(retCode);
   }
   

   
   I would prefer for better reading
   

   if (tableNameStr.isEmpty())
   {
      return null;
   }
   Buffer buffer = null;
   ...
   
   if (srcBuffer == null)
   {
      return new logical(false);
   }
   boolean retCode = false;
   

3. You don't need contains check in
   

   if(fieldsToAdd.contains(excpt[i]))
   {
      fieldsToAdd.remove(excpt[i]);
   }
   ...
   if(addedName != null && !addedName.isEmpty() && fieldsToAdd.contains(addedName))
   {
      fieldsToAdd.remove(addedName);
   }
   

4. colsAdded should probably renamed to colsAddedFrom and explicitly state in javadoc that in contains ONLY columns added thru ADD-COLUMNS-FROM. Also, it should be initialized on demand: it is used rarely.
5. I don't see a point in leaving commented out the code which "was removed because does not match the real 4GL behavior".

Stanislav Lomany wrote:

Code review branch 3811a rev 11309, 11311, 11316 regarding ADD-COLUMNS-FROM I have only code style notes:

1. queryToCheck parameter is not needed in getFieldsForBuffer and getBufferForTable.

Agree for getFieldsForBuffer but for getBufferForTable it is used inside the method.

All other notes have been resolved too.

Task branch 3811a has been updated for review to revision 11319.

Notes resolution.

but for getBufferForTable it is used inside the method.

It is always the browse query. Why pass it as a parameter?

Stanislav Lomany wrote:

but for getBufferForTable it is used inside the method.

It is always the browse query. Why pass it as a parameter?

1. For better reading(no expenses because it is reference, not object copy)
2. For possible further usage with other query

See #2921-15 for the code review.

Task branch 3811a has been updated for review to revision 11323.

Notes resolution. EDIT-* related widget implementation are moved to new ClipboardHelper class.

In 3811a rev 11323, I see no reason for the new ClientExports APIs to return a Boolean instance instead of a boolean native value. Also, there is no NPE protection where thse APIs are used, and also I don't see how the ThinClient side can return a null value.

Eugenie: if I'm correct in the above, please change the new ClientExports APIs to return a boolean value.

Constantin Asofiei wrote:

In 3811a rev 11323, I see no reason for the new ClientExports APIs to return a Boolean instance instead of a boolean native value.

Task branch 3811a has been updated for review to revision 11324.

Changed return values from Boolean to boolean.

Also, there is no NPE protection where thse APIs are used, and also I don't see how the ThinClient side can return a null value.

Please clarify. I do not understand where is the missing NPE protection. The code in ThinClient:

   public boolean editCopySelection(int id, boolean cut)
   {
...      
      if (w instanceof TextSelection)

Eugenie Lyzenko wrote:

Task branch 3811a has been updated for review to revision 11324.

Changed return values from Boolean to boolean.

Changes look good.

Also, there is no NPE protection where thse APIs are used, and also I don't see how the ThinClient side can return a null value.

I meant when you used Boolean return type; but as you changed it to boolean, there's no need for NPE checks.

Code Review Task Branch 3811a Revision 11324

I'm good with the changes.

Constantin: Are you OK with the vertical positioning changes?

Hynek: Are you OK with the MIN-HEIGHT-CHARS and MAX-HEIGHT-CHARS changes? I'm surprised that more runtime support was not needed for those.

Code review MIN-HEIGHT-CHARS and MAX-HEIGHT-CHARS changes in 3811a.

• Reference to MAX-WIDTH-CHARS in the javadoc for BrowseInterface.setMaxHeightChars() methods
• Reference to MIN-WIDTH-CHARS in the javadoc for BrowseInterface.setMinHeightChars() methods
• setMaxHeight(double) and setMinHeight(double) are redundant when setMaxHeight(NumberType) and setMinHeight(NumberTypes) overloads exist

In methods_attributes.rules

               <!-- MAX-HEIGHT-CHARS support for browse -->
               <rule>ftype == prog.kw_max_h_c
                  <rule>htype == prog.kw_browse
                     <action>hwrap = "Browse"</action>
                     <action on='false'>hwrap = "Window"</action>
                  </rule>
                  <action>methodText = "getMaxHeightChars"</action>
                  <rule>isAssign
                     <action>methodText = "setMaxHeightChars"</action>
                  </rule>
               </rule>

the comment is a bit misleading, as the rule also covers Window widget. Besides, for MIN-HEIGHT-CHARS the rule structure is slightly different:

               <rule>ftype == prog.kw_min_h_c and htype == prog.kw_browse
                  <action>hwrap = "Browse"</action>
                  <action>methodText = "getMinHeightChars"</action>
                  <rule>isAssign
                     <action>methodText = "setMinHeightChars"</action>
                  </rule>
               </rule>

               <rule>ftype == prog.kw_min_h_c and htype != prog.kw_browse
                  <action>hwrap = "Window"</action>
                  <action>methodText = "getMinHeightChars"</action>
                  <rule>isAssign
                     <action>methodText = "setMinHeightChars"</action>
                  </rule>
               </rule>

It would be better to use the same pattern for clarity for both MIN-HEIGHT-CHARS and MAX-HEIGHT-CHARS.

Hynek Cihlar wrote:

Code review MIN-HEIGHT-CHARS and MAX-HEIGHT-CHARS changes in 3811a.

• Reference to MAX-WIDTH-CHARS in the javadoc for BrowseInterface.setMaxHeightChars() methods
• Reference to MIN-WIDTH-CHARS in the javadoc for BrowseInterface.setMinHeightChars() methods
• setMaxHeight(double) and setMinHeight(double) are redundant when setMaxHeight(NumberType) and setMinHeight(NumberTypes) overloads exist

In methods_attributes.rules
[...]

the comment is a bit misleading, as the rule also covers Window widget. Besides, for MIN-HEIGHT-CHARS the rule structure is slightly different:
[...]

It would be better to use the same pattern for clarity for both MIN-HEIGHT-CHARS and MAX-HEIGHT-CHARS.

Task branch 3811a has been updated for review to revision 11325.

Notes resolution.

Task branch 3811a has been rebased with trunk 11302, new revision is 11326.

Can I retest and if no regression - commit to the trunk?

Yes, go ahead.

Greg Shah wrote:

Yes, go ahead.

OK. The conversion test has been started. Then runtime.

Conversion testing completed. The generated sources are identical.

Starting the runtime tests for 3811a.

The main regression part completed without regressions. Waiting for CTRL-C to be done.

Can I merge 3811a into trunk if everything will be OK?

Yes, go ahead.

Greg Shah wrote:

Yes, go ahead.

Testing completed. The results: 3811a_11326_32748a0_20190416_evl.zip.

No regressions found. Going to merge 3811a into trunk in 5 min.

Branch 3811a was merged to trunk as revno 11303 then it was archived.

Please confirm that everything is complete except for MANUAL-HIGHLIGHT, for which we need 4GL testcases.

For the MANUAL-HIGHLIGHT 4GL testcases:

• We understand that some widgets do not honor MANUAL-HIGHLIGHT. The idea is that for some widget types you can set MANUAL-HIGHLIGHT to TRUE, but there is no difference in how it draws or behaves. Which widgets have this result? I think BUTTON may be one of them.
• For each other widget type (which does honor MANUAL-HIGHLIGHT), please write testcases that:
  • Demonstrate how the 4GL changes its drawing/behavior, without any 4GL code that draws manually. This should show the absence of the 4GL behavior (i.e. the behavior/drawing that is turned off by setting MANUAL-HIGHLIGHT).
  • Show example 4GL code that does manually draw a custom highlight.

Greg Shah wrote:

Please confirm that everything is complete except for MANUAL-HIGHLIGHT, for which we need 4GL testcases.

Yes, I think all done except this. And COMBO-BOX representation for browse widget cell which is another task I guess.

The manual-highlight testcase is in the new testcases project (see ui/manual_highlight/w-manualhighlight.w and resources/fwd.png.

SELECTABLE and MANUAL-HIGHLIGHT attributes are available only for GUI widgets.
MANUAL-HIGHLIGHT attribute is honored by the following GUI widgets:

Button, 
Combo-box, 
Editor, 
Fill-in, 
Frame, 
Image, 
Literal, 
Radio-set,
Rectangle, 
Selection-list, 
Slider, 
Text, 
Toggle-box

The effects of SELECTABLE and MANUAL-HIGHLIGHT attributes on widget behavior:

1. If SELECTABLE=FALSE for a widget:

- MANUAL-HIGHLIGHT attribute is ignored.
  - No highlight indicator is draw for the widget.
  - SELECTION and DESELECTION events are not fired.

2. If SELECTABLE=TRUE for a widget:

- if MANUAL-HIGHLIGHT=FALSE the standard highlight indicator is draw for widget (a rectangle around the widget).

- if MANUAL-HIGHLIGHT=TRUE no standard highlight indicator is draw for widget. User is allow to draw a custom highlight indicator.
   - SELECTION and DESELECTION events are fired. Inside the triggers for these events is the location where the statements for drawing a custom highlight indicator should be placed.
   - Inside ON SELECTION trigger block the custom highlight should be draw and inside ON DESELECTION trigger block the custom highlight should be clean-up.
   - Drawing a custom highlight indicator is optional. If the user does not provide statements to draw a custom highlight indicator, then no highlight is draw for the widget.

Basically on this task we have to impliment only the management of standard highlight drawing, depending on SELECTABLE and MANUAL-HIGHLIGHT values, as already has been described.

Caused by: org.hibernate.exception.GenericJDBCException: Invalid value "en_US_P2J" for parameter "collation"; SQL statement:

See Text Collator

These are good findings. Please do fix these bugs as part of this work.

Created task branch 3811b from FWD trunk revision 11327.

Are you describing a behavior in 4GL or FWD?

Greg Shah wrote:

Are you describing a behavior in 4GL or FWD?

In FWD

OK, then there are more fixes needed. Thanks.

Please do.

Please do check the testcases/uast/slider/*.p programs to confirm that your findings are correct. It seems strange that we would have implemented a more complicated approach if it was unnecessary.

Constantin/Eugenie: Please review the changes in 3811b.

Greg Shah wrote:

Constantin/Eugenie: Please review the changes in 3811b.

I have no objections in general. The questions:
1. The 11330 re-enables the select/deselect events for skip widget. The reason? Did you test whether this change cause regressions for skip widget becomes selected and has selection border around? The skip widget can never be selected.
2. The slider widget has been significantly refactored(11331-11333). Have you tested it with standalone GUI slider tests, in particular for frame 3D mode in classic theme?

Eugenie Lyzenko wrote:

1. The 11330 re-enables the select/deselect events for skip widget. The reason? Did you test whether this change cause regressions for skip widget becomes selected and has selection border around? The skip widget can never be selected.

Eugenie, is not obvious for me how this is happening - can you clarify a little?

Marius, please update the copyright year to 2019, for the files you changed. Also, have you tested with the Web client? Otherwise, beside making sure this is not breaking anything else in GUI (like direct manipulation for other widgets), I'm good with the changes.

Constantin Asofiei wrote:

Eugenie Lyzenko wrote:

1. The 11330 re-enables the select/deselect events for skip widget. The reason? Did you test whether this change cause regressions for skip widget becomes selected and has selection border around? The skip widget can never be selected.

Eugenie, is not obvious for me how this is happening - can you clarify a little?

If widget selection is happening via mouse click or rectangular selection - the widget become selected and draws the selection frame around itself. The skip widget is not involved in this process, it can not be selected or deselected. Sending DESELECT message to this widget is useless(and CPU consuming) at least.

Eugenie Lyzenko wrote:

Greg Shah wrote:

Constantin/Eugenie: Please review the changes in 3811b.

I have no objections in general. The questions:
1. The 11330 re-enables the select/deselect events for skip widget. The reason? Did you test whether this change cause regressions for skip widget becomes selected and has selection border around? The skip widget can never be selected.
2. The slider widget has been significantly refactored(11331-11333). Have you tested it with standalone GUI slider tests, in particular for frame 3D mode in classic theme?

1. DESELECT message is sent only to widgets which are selectable and selected. SKIP and SPACE widgets
are not selectable and never selected and they never receive a DESELECT message. I tested and SKIP widget is never selected.
BTW in 4gl it is not possible to set SKIP widget SELECTABLE attribute because the SKIP widget cannot be defined as a variable having a name.
Trying to do that:

ASSIGN SKIP:SELECTABLE in FRAME FRM-1 = TRUE.

result in an error:
  ** Unknown Field or Variable name - SKIP. (201)
  **  Could not understand line 22. (196)

2. The changes in draw method for GUI Slider widget are related to layout only, setting the location of slider components only.
Basically the selection frame position (coordinates) is set and the ruler component position relative to the selection frame.
The drawing code for slider components: ruler, thumb button, tic marks, selection frame has not been changed, I didn't touch the code.
So the widget looks like in the original implementation but now we can set the position of slider components with 1 pixel granularity.
I tested the changes using UAST test from slider/slider-*.p and I found no problems.
But those tests does not cover all scenarios. For example the original implementation does not work properly with provided UAST test
manual-highlight/w-manualhighlight.w for selection/deselection. I fixed a lot of bugs for selection/deselection widgets, among them the slider layout.
With the new changes in place all UAST tests looks good.

Constantin Asofiei wrote:

Eugenie Lyzenko wrote:

1. The 11330 re-enables the select/deselect events for skip widget. The reason? Did you test whether this change cause regressions for skip widget becomes selected and has selection border around? The skip widget can never be selected.

Eugenie, is not obvious for me how this is happening - can you clarify a little?

Marius, please update the copyright year to 2019, for the files you changed. Also, have you tested with the Web client? Otherwise, beside making sure this is not breaking anything else in GUI (like direct manipulation for other widgets), I'm good with the changes.

1. I'll update the copyright year to 2019 in the changed files.

2. I tested only with Swing GUI client not with the Web GUI client. How could I do tests with Web GUI client? Where I can find instructions on how to do that?

Task branch 3811b has been rebased with trunk 11330, new revision is 11338.

I tested only with Swing GUI client not with the Web GUI client. How could I do tests with Web GUI client? Where I can find instructions on how to do that?

Have you tried getting the Hotel GUI running? These instructions were written for external use. Instead of downloading the zip, you should check it out using bzr co ~/secure/code/p2j_repo/samples/hotel_gui. The rest of the instructions should be pretty close to right.

Once you have that working, you can look at the changes the prepare script made to the directory and make similar changes to the directory in testcases/simple/server/ to get your samples running in the web client.

We also have a Web Client Setup. It is mostly up to date, but not 100%.

Nice catch!

A few weeks ago, Stanislav reported something similar. He said that the arrow buttons (e.g. on scrollbars) did not work in the classic theme in trunk. I hope this is the same problem.

Are there any remaining known issues for 3811b?

Greg Shah wrote:

Nice catch!

A few weeks ago, Stanislav reported something similar. He said that the arrow buttons (e.g. on scrollbars) did not work in the classic theme in trunk. I hope this is the same problem.

Are there any remaining known issues for 3811b?

I'm working to fix the selectable widgets behavior on mouse click and keyboard events. So far I fixed the mouse click.

Marius, Please do fix that slider background issue in 3811b.

I never worked with such kind of images (.svg) Which tools are you using to create such resources?

Marius Gligor wrote:

I never worked with such kind of images (.svg) Which tools are you using to create such resources?

I have good experience with Inkscape.

Marius Gligor wrote:

Branch 3811b is ready for a code review.

I'm OK with the changes. If you didn't, please do some tests with Hotel GUI in Web mode.

When we select the RadioSet widget on Web GUI client the background color is set to Red but the radio buttons labels are draw over, with initial background color. Clicking again on the widget at one time the entire background is painted in Red! (see attached pictures). For Classic UI theme the background is never paint in Red entirely! I experienced the same behavior in Firefox and @Chrome2 web browsers.

I guess it makes sense to fix this now, please go ahead.

Marius Gligor wrote:

When next draw occur, the background color became RED, but the cached draw is send to JS.

I assume at some point AbstractGuiDriver.setColor is called? In any case, I'd like to see the stacktrace which calls the driver API to set this RED background color.

We might call this API outside of the normal driver APIs which are used for drawing (where the recording is used).

Constantin, using your hint I quickly fixed the bug. Many thanks!
Indeed, the AbstractGuiDriver.setColor MUST be called but this is not happen for RadioButton widget.

In p4gl RADIO-BUTTONS is an attribute for a RADIO-SET widget. In FWD, RADIO-BUTTONS attribute is implemented as a RadioButon widget in the client side.
The BGCOLOR is changed for RADIO-SET widget and the drawing is very simple, a rectangle filled with the BGCOLOR.
AbstractGuiDriver.setColor is called to set the fill color and everything works properly.

For a RadioButton widget only the button image and button label are rendered, but no call to AbstractGuiDriver.setColor!
RadioButton widget, share the same configuration as the RADIO-SET widget on which belong, and the colors are similar resolved.
RadioButton background color MUST be the same as the RADIO-SET background color.

In order to fix the bug I added a call to AbstractGuiDriver.setColor on each <ThemeName>Theme.drawRadioButton() method where
the RadioButton widget is draw:

   public void drawRadioButton(RadioButtonGuiImpl rb,
                               GuiDriver gd,
                               GuiColorResolver gc,
                               GuiFontResolver gf,
                               int x,
                               int y,
                               int width,
                               int height,
                               boolean focused,
                               boolean hover)
   {
      // set background color
      gd.setColor(gc.bgColor);

With these changes in place the bug is now fixed and all tests successfully passed.
These changes are mandatory when graphical cache is enabled, but nevertheless they works when cache is disabled as well.

My changes has been committed in branch 3811b rev 11349, 11350

Great! Please run back through the GUI testing (Hotel GUI and your testcases).

Constantin: Any concerns with the final changes?

Marius Gligor wrote:

Constantin, using your hint I quickly fixed the bug. Many thanks!
Indeed, the AbstractGuiDriver.setColor MUST be called but this is not happen for RadioButton widget.

I don't understand how your change works - a setColor call should be followed by a call to actually draw the background. I don't see anything like this in drawRadioButton. Did you re-enable the caching mechanism when you tested?

For example, the classic theme has code like this:

         gd.setColor(disabledTextColorBk);
         gd.drawString(label, xt + 1, yt + 1);

Do idea is, when setColor is called, it should be followed by a call to actually draw something with that color.

Constantin Asofiei wrote:

Marius Gligor wrote:

Constantin, using your hint I quickly fixed the bug. Many thanks!
Indeed, the AbstractGuiDriver.setColor MUST be called but this is not happen for RadioButton widget.

I don't understand how your change works - a setColor call should be followed by a call to actually draw the background. I don't see anything like this in drawRadioButton. Did you re-enable the caching mechanism when you tested?

For example, the classic theme has code like this:
[...]
which sets the color before calling a primitive to draw something.

Do idea is, when setColor is called, it should be followed by a call to actually draw something with that color.

After setting the background color, the radio button image and radio label are drawing. (please take a look over drawRadioButton() method)
Also a dotted rectangle is draw when the widget is selected.
As I understood the graphical cache store data per widget Id and all drawing ops on drawRadioButton() are cached.
So all graphical ops within drawRadioButton() are recorded including my changes to set the color.
When DRAW_CACHED is sent to JS means that a cached image should be draw. The cached image is an image recorded by applying graphical ops.
Is this correct or is a misunderstanding of graphical cache concept?

Marius Gligor wrote:

After setting the background color, the radio button image and radio label are drawing. (please take a look over drawRadioButton() method)
Also a dotted rectangle is draw when the widget is selected.

I'm confused because you just make the sequence of draw operations to reflect the new background color, but this isn't actually used in drawRadioButton() to draw the background. Your change makes the draw sequence to be unique in terms that it will not use an incorrectly cached image. My problem is that I don't understand who draws the background for the radio button. I found only this code in RadioSetGuiImpl:

         // fill the background
         gd.setColor(gc.bgColor);
         gd.fillRect(0, 0, d.width, d.height);

As I understood the graphical cache store data per widget Id and all drawing ops on drawRadioButton() are cached.
So all graphical ops within drawRadioButton() are recorded including my changes to set the color.
When DRAW_CACHED is sent to JS means that a cached image should be draw. The cached image is an image recorded by applying graphical ops.
Is this correct or is a misunderstanding of graphical cache concept?

Yes, this is correct; a small note: the cached image used is always the one 'most inclusive': i.e. if is determined that the cached image for the entire parent frame can be used, then that will be used, and not for its individual widgets.

On Swing and Web GUI driver the bgcolor is set when radio-set is drawing and seems to be used when draw radio-button as well.
When the radio button label is draw the bgcolor of radio-set is used.
Probably the GuiDriver instance is the same since the radio button is draw within radio-set in method drawInt()
What do you think about that?

I just tested. I added a simple System.out.println(gd); when RadioSetGuiImpl and RadioButtonGuiImpl widgets are created (constructor)
Then running my app with Swing GUI Client I captured from stdin:

 
com.goldencode.p2j.ui.client.gui.driver.swing.SwingGuiDriver@4b5189ac
com.goldencode.p2j.ui.client.gui.driver.swing.SwingGuiDriver@4b5189ac
com.goldencode.p2j.ui.client.gui.driver.swing.SwingGuiDriver@4b5189ac
com.goldencode.p2j.ui.client.gui.driver.swing.SwingGuiDriver@4b5189ac
com.goldencode.p2j.ui.client.gui.driver.swing.SwingGuiDriver@4b5189ac
com.goldencode.p2j.ui.client.gui.driver.swing.SwingGuiDriver@4b5189ac
com.goldencode.p2j.ui.client.gui.driver.swing.SwingGuiDriver@4b5189ac
com.goldencode.p2j.ui.client.gui.driver.swing.SwingGuiDriver@4b5189ac
com.goldencode.p2j.ui.client.gui.driver.swing.SwingGuiDriver@4b5189ac
com.goldencode.p2j.ui.client.gui.driver.swing.SwingGuiDriver@4b5189ac
com.goldencode.p2j.ui.client.gui.driver.swing.SwingGuiDriver@4b5189ac
com.goldencode.p2j.ui.client.gui.driver.swing.SwingGuiDriver@4b5189ac
com.goldencode.p2j.ui.client.gui.driver.swing.SwingGuiDriver@4b5189ac
com.goldencode.p2j.ui.client.gui.driver.swing.SwingGuiDriver@4b5189ac
com.goldencode.p2j.ui.client.gui.driver.swing.SwingGuiDriver@4b5189ac
...

Constantin Asofiei wrote:

Marius Gligor wrote:

After setting the background color, the radio button image and radio label are drawing. (please take a look over drawRadioButton() method)
Also a dotted rectangle is draw when the widget is selected.

I'm confused because you just make the sequence of draw operations to reflect the new background color, but this isn't actually used in drawRadioButton() to draw the background. Your change makes the draw sequence to be unique in terms that it will not use an incorrectly cached image. My problem is that I don't understand who draws the background for the radio button. I found only this code in RadioSetGuiImpl:
[...]
Why isn't there a similar code for individual radio-button drawing?

Yes, the background for radio-button is painting one time inside radio-set. No code to draw BG for each radio-button. The idea was the radio-button is a part of the enclosing object(radio-set) or even if standalone - it will use the background of the object behind.

Eugenie Lyzenko wrote:

Constantin Asofiei wrote:

Marius Gligor wrote:

After setting the background color, the radio button image and radio label are drawing. (please take a look over drawRadioButton() method)
Also a dotted rectangle is draw when the widget is selected.

I'm confused because you just make the sequence of draw operations to reflect the new background color, but this isn't actually used in drawRadioButton() to draw the background. Your change makes the draw sequence to be unique in terms that it will not use an incorrectly cached image. My problem is that I don't understand who draws the background for the radio button. I found only this code in RadioSetGuiImpl:
[...]
Why isn't there a similar code for individual radio-button drawing?

Yes, the background for radio-button is painting one time inside radio-set. No code to draw BG for each radio-button. The idea was the radio-button is a part of the enclosing object(radio-set) or even if standalone - it will use the background of the object behind.

Indeed, without graphical cache the RadioSet widget work properly.
The BG color is set when RadioSet is draw and used also when the RadioButton is draw.
Using the graphical cache, the recorded image for RadioButton has no BG ops since this done in another widget, RadioSet.
Adding a set BG ops to RadioButton as well, the recorded image is now correct.
We can do that only when the graphical cache is enabled if you consider that, but we have to do more changes to check the value of graphicsCached in GuiWebDriver.
Maybe a better solution is to add more description to javadoc of drawRadioButton method, just to reflect all aspects discussed here regarding this topic.

Marius, OK, I understand now - the issue was that for FWD, the radio-buttons are individual widgets, and we cache the image for them, without any info related to the background color.

So the changes are OK. Please merge the branch to trunk.

Marius Gligor wrote:

Maybe a better solution is to add more description to javadoc of drawRadioButton method, just to reflect all aspects discussed here regarding this topic.

And do update the javadoc before merging.

In class GuiWebSocket.java I found the following method:

   /**
    * Compute an MD5 hash for the given message.
    * 
    * @param    message
    *           The message with the drawing ops.
    * 
    * @return   A 16-byte array with the MD5 hash.  If the hash could not be computed, return
    *           null.
    */
   private byte[] md5Hash(byte[] message)
   {
      try
      {
         if (MD5 == null)
         {
            MD5 = MessageDigest.getInstance("MD5");
         }

         byte[] drawHash = MD5.digest(message);
         for (int i = 0; i < drawHash.length; i++)
         {
            drawHash[i] = (byte) (drawHash[i] & 0xFF);
         }
         return drawHash;
      }
      catch (NoSuchAlgorithmException e)
      {
         e.printStackTrace();
      }

      return null;
   }

What I don't understand here is the for loop because drawHash is a byte array and byte in Java is 8 bits in size.
The for loop has no effect on drawHash array!

Marius Gligor wrote:

In class GuiWebSocket.java I found the following method:

[...]

What I don't understand here is the for loop because drawHash is a byte array and byte in Java is 8 bits in size.
The for loop has no effect on drawHash array!

I don't recall why I added that; but I agree, the loop is a no-op - remove it and do some tests, if caching works properly (you will see messages in the JS console).

FYI, the MANUAL-HIGHLIGHT entry in rules/gaps/expressions.rules was not updated:

<rule>attrs.put(prog.kw_man_high, rw.cvt_lvl_full       | rw.rt_lvl_stub)</rule>

I will make the change in 4069a (from rw.rt_lvl_stub to rw.rt_lvl_full).