Project

General

Profile

Feature #3880

enhanced browse 3rd phase of improvements

Added by Greg Shah over 5 years ago. Updated over 2 years ago.

Status:
WIP
Priority:
Normal
Target version:
-
Start date:
Due date:
% Done:

0%

billable:
No
vendor_id:
GCD

right.png (645 Bytes) Stanislav Lomany, 05/07/2020 05:00 PM

artefacts.jpg (4.35 MB) Stanislav Lomany, 05/18/2020 01:43 PM

browse-simple.p Magnifier (747 Bytes) Stanislav Lomany, 06/04/2020 07:50 PM

font-size.png (41.8 KB) Stanislav Lomany, 06/10/2020 02:55 PM

browses.png (6.3 KB) Stanislav Lomany, 06/10/2020 05:44 PM

br2.png (6.79 KB) Stanislav Lomany, 06/11/2020 05:57 AM

br1.png (6.19 KB) Stanislav Lomany, 06/11/2020 05:57 AM

example.png (1.94 KB) Stanislav Lomany, 06/11/2020 06:28 AM

gear.png (4.45 KB) Stanislav Lomany, 06/11/2020 02:29 PM

fontdiff.png (27.9 KB) Stanislav Lomany, 06/16/2020 05:30 AM

EnhancedBrowseConfigInlocalStorage.png (153 KB) Sergey Ivanovskiy, 06/19/2020 04:16 PM

EnhancedBrowseConfigInlocalStorage_stored_for_a_user.png (126 KB) Sergey Ivanovskiy, 06/19/2020 04:27 PM

EnhancedBrowseConfigInlocalStorage_stored_for_a_user2.png (117 KB) Sergey Ivanovskiy, 06/19/2020 07:10 PM


Related issues

Related to User Interface - Feature #3261: enhanced browse that can optionally selected as a replacement for the default ABL browse Closed
Related to User Interface - Feature #4286: support user-specific offline storage that is provided by the client's UI driver Closed
Related to User Interface - Feature #4517: optionally back the 4GL features for Registry access with the user-specific offline storage features Closed
Related to User Interface - Feature #4854: origin affinity Closed

History

#1 Updated by Greg Shah over 5 years ago

  • Related to Feature #3261: enhanced browse that can optionally selected as a replacement for the default ABL browse added

#4 Updated by Greg Shah over 5 years ago

The first two phases of effort are documented in #3261 and #3706.

The following items need to be resolved/added/improved:

  1. Persistence of User Customization
    1. Change the per-user storage/retrieval of enhanced browse config to use the #4286 user-specific offline storage API. All user-level changes always store in the user-specific offline storage. Admin changes store only in the directory.
    2. Make "Change Layout UI" configuration parameter take three possible states: DISABLED, ENABLED_ALL_USERS, ENABLED_ADMIN_USERS. ENABLED_ALL_USERS should be the default.
    3. As a simpler alternative to the current enhanced browse configuration, please create a single directory value to turn on all enhanced browse features.
    4. Implement proper "reset to default state" for the new security paradigm.
  2. Export
    1. Output a better title for browse exports. The current code uses "Browse Report", which means it has nothing to do with the actual business purpose of the report. Most users have no idea what a "browse" is. It won't make any sense to user. What can we read from the browse or frame state to create a better description? We probably also need to allow the user to override that title.
    2. Considering the latest changes in reports, keys used for report caching are not accurate any more. Use the same keys as for enhanced configurations.
    3. Replace JasperReports export approach with one based on Apache POI which will eliminate the page-related approach in favor of a cell and row based output that customers would expect to see.
    4. Allow the default template for export to be customized with a logo or other static elements.
  3. Look and Feel
    1. Web column/row resize issues.
    2. When the cells font is altered, font height (and therefore row height) may be incorrectly calculated for some fonts in web.
    3. Resolve the conflict between enhanced browse popup menus and any popup menus defined by the original 4GL application. The upper left corner of the browse has no purpose, in enhanced mode it will be drawn as a gear icon instead of the 3D placeholder that exists there now pressing the gear icon will cause the enhanced browse popup to display the regular 4GL popup menu processing can remain for the rest of the browse (right click opens any browse-specific context menu and if none exists, then the parent hierarchy popup menu is opened). In other words, we will separate the 4GL code popup menu from the enhanced browse popup menu.
    4. Improve the usability of sorting/filtering mode. We must be able to simplify some steps. Today it is lots of extra mouse clicking/moving to do this, with popup menus and dialogs.
      • Instead, we want to keep the filtering and sorting controls always visible. It is OK to create extra space in/under the header for this. It is optimal for all controls to be always visible but if absolutely needed it can auto-expand when the user clicks into it (like a "drawer").
      • By default, do not prompt the user to apply the sort. Add a configuration option that allows the customer to force the prompt to appear.
    5. Improve the look of the filtering input fields. This includes improvements to the sorting arrows and the filtering X.
    6. Laggy popup menu selection in classical swing theme.
    7. Disallow user re-ordering of the leftmost NUM-LOCKED-COLUMNS.
  4. Additional Features (these come from #3765-17 and I have considered and currently rejected the items in #3765-14)
    1. Add support for scroll-notify and mouse-select-dblclick events. This is not enhanced browse, but should always be supported.
    2. column order
    3. column title
    4. column visibility
    5. number of decimals
    6. add total/min/max/etc value which is displayed under the browse for the column
    7. number of fixed columns
    8. number of lines for column header (?)
  5. 4GL Syntax Enhancements
    1. Provide a 4GL syntax option to disable user-controlled sorting for that specific browse instance.
    2. Add a DISABLE-ROW-STRIPING browse option to disable the row striping enhancement on a per-browse basis.
    3. Add a DISABLE-CFG-EDIT browse option to disable the ability of all users (including admins) to change any of the otherwise customizable configuration on a per-browse basis. See #5092-130 and #5092-133 through #5092-134. This means that all of the colors, fonts, sizing, column hiding, reorder columns... would be treated as read-only with no ability for a user to modify the values. The values would be set from the directory as normal OR would be overridden by any 4GL syntax enhancements used.
    4. Add a <browse_widget>:UNIQUE-KEY attribute to provide 4GL control over the mechanism to identify unique browse instances in the directory. See #5092-54.
      • Add a read/write character attribute BROWSE:UNIQUE-KEY. The default value should be "".
      • For dynamic browse, let's not use the columns to create the key for the directory lookup. Since columns can be added later, it seems that we should rely upon the procedure, browse name and the unique-key to properly read the configuration.
      • As columns are dynamically added, apply the column level settings.
      • Possible fallback to use the column names if the UNIQUE-KEY is not set.
    5. Add DB-SORT-FIELD-MAPPING and DB-FILTER-FIELD-MAPPING browse column attributes. This allows the customization of sorting, filtering from the default. This is useful for calculated columns. It allows one to see a column, sort by a second column and filter by third. Complex expressions are not supported at this time, just simple mappings of one field to another (potentially hidden) field. See #5092-59 through #5092-108, #5092-111 through #5092-115, #5092-121 through #5092-124.
    6. Add the my-browse-column:ALIGNMENT attribute to control column alignment.
      • This should support values of LEFT, RIGHT and CENTER. We do not need JUSTIFY.
      • If not otherwise specifically set there are some defaults based on the data type.
        • left: character, date, datetime, datetime-tz, logical
        • right: anything numeric (int, int64, decimal, recid)
      • Column label and data obey the same alignment.
      • The need for these changes comes from #3765-100 (and #3765-116 through 119). I think the customer code should be modified to use this new feature. Please create the proper diffs, test them and send them to the customer.
    7. Expose all of the Change Layout capabilities through 4GL syntax (the idea is that developers can use these same features to implement this customization in their 4GL code).
    8. Add export events (report begin, report end, page begin and page end) which can be configured using 4GL browse-level options. When configured, the browse will PUBLISH the event and any subscribers will have the opportunity to customize the export output. The signature for the event will need to include enough state that the subscriber can do something useful. For example, the browse handle and the report handle probably both need to be passed. Output of a report-level header/footer and customizing the page level header and footer are examples that should be possible. The programmer would need to be able to edit the values in some way, so this probably depends upon the other enhanced features being exposed as new 4GL syntax.

#5 Updated by Greg Shah over 4 years ago

  • Related to Feature #4286: support user-specific offline storage that is provided by the client's UI driver added

#6 Updated by Greg Shah over 4 years ago

  • Status changed from New to WIP
  • Assignee set to Stanislav Lomany

#7 Updated by Stanislav Lomany about 4 years ago

Guys, I need to get user id (logged in) on the client side. I'm planning to get it from the server, but I wonder when the value cached on the client should be invalidated. What is the best place on the client side to perform invalidation, where a user is logged in or logged out?

#8 Updated by Stanislav Lomany about 4 years ago

  1. Output a better title for browse exports. The current code uses "Browse Report", which means it has nothing to do with the actual business purpose of the report. Most users have no idea what a "browse" is. It won't make any sense to user. What can we read from the browse or frame state to create a better description? We probably also need to allow the user to override that title.

I don't think we'll be able to "guess" a good title for a report. In most cases it'll be a browse without title inside a no-title frame inside a no-title frame inside a frame and so on.
Currently we can set report title using

browse brws:report:set-report-param("CustomTitle", "Custom Browse Report").

Maybe creation of an option for DEFINE BROWSE (REPORT-TITLE) may improve the situation. Also we can rename "Browse Report" to "Report" or "Table Report" or "Data Report".

#9 Updated by Greg Shah about 4 years ago

Default report title, in order of preference:

  1. User-specified custom title.
  2. If the browse has a title, use it.
  3. If the containing frame has a title, use it.
  4. "Report"

Let's focus on allowing the user to customize it. That really resolves the issue. What can be done for this?

#10 Updated by Greg Shah about 4 years ago

Please check in your changes to 4335a.

#11 Updated by Stanislav Lomany about 4 years ago

Implement proper "reset to default state" for the new security paradigm.

Every user (including admins) has three levels of configurations to consider:
  1. Default browse state (no enhanced config).
  2. "All users" config.
  3. User config.

So when user "reverts to default state", which state is considered to be default - "All users" configuration or "No enhanced configuration"? Should a user be able to revert to "No enhanced configuration" state bypassing "All users" configuration?

Admins can only revert to "No enhanced configuration" state.

#12 Updated by Greg Shah about 4 years ago

For now, I think it is enough for the user to be able to clear their own customizations. I think this corresponds to your "All users" config. Call the option "Reset to Default". The "all users" configuration (if any) is the default.

#13 Updated by Greg Shah about 4 years ago

Stanislav Lomany wrote:

Guys, I need to get user id (logged in) on the client side. I'm planning to get it from the server, but I wonder when the value cached on the client should be invalidated. What is the best place on the client side to perform invalidation, where a user is logged in or logged out?

It really depends on what you mean by "login" or "logout".

The 4GL doesn't have a well defined and comprehensive security model. Although there are some security features, securing an application is essentially a "do it yourself" (DIY) approach.

When the FWD session is started, there will be some form of authentication, which may set a default FWD account or otherwise be non-interactive (e.g. using a certificate). If the application has been modified to use the FWD security model instead of its own DIY approach, then the FWD session authentication is also the application login and the FWD session termination is logout. Most converted applications don't use this because it is extra effort (custom conversion rules and code changes and planning) to rework the security model. A notable exception is the large ChUI application which we use for ChUI regression testing. In this approach, login/logout is not useful for invalidation of the userid.

In the DIY model, the userid can be set (and later changed to some degree) after the FWD session is started. I think it consists of the following features:

  • SETUSERID() built-in function
  • SET-DB-CLIENT() built-in function
  • SECURITY-POLICY:SET-CLIENT() method
  • default OS user account (if none of the above are successfully called, then the OS account name is reported as the userid)

Igor/Ovidiu/Constantin: Am I missing anything here? Did I misstate anything?

If we need some kind of invalidation/change notification, it would have to be an event that was generated on the server side when one of the above mechanisms successfully change the userid.

#14 Updated by Stanislav Lomany about 4 years ago

Let's focus on allowing the user to customize it. That really resolves the issue. What can be done for this?

I think we can add an option to the Export menu, next to the Export to PDF, Export to XLS etc. Like Change report title....
Do we want to have "user-specific" and "all users" settings as with enhanced browse configurations?

#15 Updated by Greg Shah about 4 years ago

Do we want to have "user-specific" and "all users" settings as with enhanced browse configurations?

Yes, but as with everything else, there should only be a single, simple menu item. If it is an admin user, then the changes should automatically be saved to the directory for all users. If it is a non-admin user, then changes are saved to the offline storage.

#16 Updated by Stanislav Lomany about 4 years ago

Regarding storage of the "report title" parameter. I think I'll add it to the enhanced browse configuration. The only issue here is that I'll think I'll add a flag that indicates if the "layout part" of the enhanced configuration is active.

<node class="enhancedBrowseConfig" name="1">
   <node-attribute name="proc_name" value="browse.p"/>
   <node-attribute name="browse_name" value="br"/>
   <node-attribute name="user_name" value="adminbogus"/>

   <node-attribute name="report_title" value="The Report"/>  <----------
   <node-attribute name="layout_active" value="true"/>       <---------- 

   <node-attribute name="fgcolor" value="#8080ff"/>
   <node-attribute name="font" value="DejaVu Sans,11,false,false,false"/>
   ...
   <node class="container" name="columns">
      <node class="enhancedColumnConfig" name="1">
         <node-attribute name="column_key" value="tt.f1"/>
         <node-attribute name="ordinal" value="1"/>
         <node-attribute name="visible" value="TRUE"/>
         ...

Let me know if you have a better idea.

#17 Updated by Greg Shah about 4 years ago

The only issue here is that I'll think I'll add a flag that indicates if the "layout part" of the enhanced configuration is active.

What is the purpose of this flag?

In general, when a customer enables enhanced browse, they are expecting all the features to be present. It will be unusual for a customer to want to disable specific enhanced browse features across the entire application. They will certainly want to set some defaults, especially in regard to the look and feel. But the reason we are looking for a simple flag for enabling enhanced browse is to make it easy to configure the common case.

#18 Updated by Stanislav Lomany about 4 years ago

What is the purpose of this flag?

OK, I think I don't actually don't need this flag. The issue I want to address is that after report title parameter is added, in order to determine if an enhanced configuration contains layout changes, I have to check if all browse and column parameters are null (not set).

#19 Updated by Greg Shah about 4 years ago

  • Related to Feature #4517: optionally back the 4GL features for Registry access with the user-specific offline storage features added

#20 Updated by Stanislav Lomany about 4 years ago

For enhanced browse changes created task branch 3880a from FWD trunk revision 11342.

#21 Updated by Stanislav Lomany about 4 years ago

Rebased task branch 3880a from P2J trunk revision 11344.

#22 Updated by Stanislav Lomany about 4 years ago

Greg, I have some conceptual questions about report-related enhanced parameters. Enhanced parameters can be divided into two categories: layout parameters and report parameters (right now the only report parameter is the report title, but template settings will come). So we have to decide how they co-exist. I see three options:
  1. They have different logical hierarchies, i.e. we search the hierarchy for the first applicable layout configuration and first applicable report configuration separately. But there is only one tree in storage hierarchy, which looks pretty, but we have to merge layout and report parts when saving a configuration.
  2. They have different logical hierarchies and two trees in storage hierarchy. The downside is that it is bulky.
  3. One logical hierarchy. The downside is that we cannot manage layout and report parameters separately.

What do you think is the best option?

#23 Updated by Greg Shah about 4 years ago

What are examples of "layout parameters"?

#24 Updated by Stanislav Lomany about 4 years ago

All kinds of colors and fonts, column width, order and visibility, row height.

#25 Updated by Greg Shah about 4 years ago

I assume that the layout parameters for use in both the screen/display and the reports.

Why do we want to keep things separate? Conceptually, it seems that the overall configuration for a browse is a single thing. It is OK for the layout and report parameters to be together.

Help me understand what you see as the advantage of separating things.

#26 Updated by Stanislav Lomany about 4 years ago

Help me understand what you see as the advantage of separating things.

I think you're right that it is a single thing, I forgot that UI parameters also affect reports.

#27 Updated by Stanislav Lomany almost 4 years ago

Guys, I wanted to add a field where I could input a report title. I used existing file/font/color selection frames as an example, and it turned out that:
  1. It has a dialog-specific problems. I fixed reverse typing order. The other one I noticed is cursor and selection being displayed in non-focused input field (repaint issue?).
  2. These dialogs use LineEditor which extends FillInGuiImpl. Line editor lacks horizontal scrolling functionality, i.e. if the typed text is longer than than the input field, its tail is hidden from a user. "File name" field in the file selection dialog is an example of such field. After I added horizontal scrolling functionality to LineEditor, I found that...
  3. FillInGuiImpl has problems with horizontal scrolling. When a string exceeds field's length, both FILL-IN and COMBO-BOX (entry field) 4GL widgets can demonstrate behavior 1. not matching 4GL. 2. functionally not correct from user's perspective (e.g. text is inserted not on the cursor position).
  4. Just found that selection is cleared on Ctrl+C.

I suggest to fix the problems (not sure about the first item) after I finish the browse task.

#28 Updated by Greg Shah almost 4 years ago

OK

#29 Updated by Stanislav Lomany almost 4 years ago

Considering the latest changes in reports, keys used for report caching are not accurate any more. Use the same keys as for enhanced configurations.

FYI I actually did nothing on this part, the way reporting is implemented makes the keys viable. If you know a counter-example, I would be glad to get it.

#30 Updated by Greg Shah almost 4 years ago

Stanislav Lomany wrote:

Considering the latest changes in reports, keys used for report caching are not accurate any more. Use the same keys as for enhanced configurations.

FYI I actually did nothing on this part, the way reporting is implemented makes the keys viable. If you know a counter-example, I would be glad to get it.

You were the one that told me this was an issue. That is how it got on the list. If it is no longer needed, that is fine.

#31 Updated by Stanislav Lomany almost 4 years ago

Provide a 4GL syntax option to disable user-controlled sorting for that specific browse instance.

Are we talking about disabling enhanced sorting? Is browse:DISABLE-EH-SORTING attribute is enough or do we need an DEFINE BROWSE option too?

#32 Updated by Greg Shah almost 4 years ago

Stanislav Lomany wrote:

Provide a 4GL syntax option to disable user-controlled sorting for that specific browse instance.

Are we talking about disabling enhanced sorting?

Yes. The idea is that when enhanced sorting is enabled for the system, some browse instances must not be allowed to have this feature due to the performance implications of unrestricted sorting on the given tables. For these cases, the customer wants the ability to disable the user-defined sorting on specific browse instances.

Is browse:DISABLE-EH-SORTING attribute is enough or do we need an DEFINE BROWSE option too?

I think the attribute is enough. Please make it a logical attribute named ENHANCED-SORTING. The developer can set it to false to disable it. When the feature is turned on in the directory, it will default to true.

#33 Updated by Stanislav Lomany almost 4 years ago

Provide a 4GL syntax option to disable user-controlled sorting for that specific browse instance.

Turns out we already have enhanced-sorting and enhanced-filtering attributes at browse and column level. They work the way we need.

#34 Updated by Greg Shah almost 4 years ago

Stanislav Lomany wrote:

Provide a 4GL syntax option to disable user-controlled sorting for that specific browse instance.

Turns out we already have enhanced-sorting and enhanced-filtering attributes at browse and column level. They work the way we need.

Hmm, I see that now in the wiki document Browse Widget Enhancements.

#35 Updated by Stanislav Lomany almost 4 years ago

  • This should support values of LEFT, RIGHT and CENTER.

For CENTER-align columns in editing mode, I think we can leave the fill-in left-aligned. As to me, 4GL right-aligned columns look more like center-aligned, so I don't think we should add more mess.

#36 Updated by Greg Shah almost 4 years ago

For CENTER-align columns in editing mode, I think we can leave the fill-in left-aligned.

Agreed. The CENTER alignment doesn't make sense in editing mode. It is about the normal display mode.

#37 Updated by Stanislav Lomany almost 4 years ago

Rebased task branch 3880a from P2J trunk revision 11346.

#38 Updated by Stanislav Lomany almost 4 years ago

I've found that for browse editing fields, at least for character type, a field doesn't switch to insert mode when it should, i.e. it's always in overwrite mode. I thought it's a regression, but it has been in the trunk at least for a pretty long time. Should I fix it?

#39 Updated by Greg Shah almost 4 years ago

I assume this is a quick fix, right? If so, yes.

#40 Updated by Stanislav Lomany almost 4 years ago

The need for these changes comes from #3765-100 (and #3765-116 through 119). I think the customer code should be modified to use this new feature. Please create the proper diffs, test them and send them to the customer.

Please send me instructions on this part.

#42 Updated by Stanislav Lomany almost 4 years ago

add support for scroll-notify

Do I need to add support for scroll-horizontal and scroll-vertical as well?

#43 Updated by Greg Shah almost 4 years ago

Yes, if you can do it quickly.

#44 Updated by Stanislav Lomany almost 4 years ago

Web column/row resize issues.

Here is the list of issues:
  1. Column resize:
    1. Chrome: works, however the area where the cursor becomes active (changes to resize arrow) slightly different than the effective area that allows to resize a column.
    2. Firefox: doesn't work, cursor becomes "hand", some artifacts occur with no actual result.
  2. Row resize:
    1. Chrome: cursor doesn't become active, but functionally it works.
    2. Firefox: cursor doesn't become active, on drag cursor becomes "hand", some artifacts occur with no actual result.
  3. Column drag:
    1. Chrome: works.
    2. Firefox: doesn't work, cursor becomes "hand", some artifacts occur with no actual result.

Greg, I'm not sure that I'm the best person to resolve browser-specific issues in adequate time.

#45 Updated by Greg Shah almost 4 years ago

We don't have anyone else available. Hynek and Eugenie have experience in the drag and drop support for web client. Ask them questions as needed to help you understand how our mouse dragging infrastructure works.

#46 Updated by Greg Shah almost 4 years ago

You've been looking into this for a week. At this point, I'm guessing you have enough understanding about how the mouse dragging works in the web client. If you have any remaining questions, please post the list ASAP.

I'd also like to know how close you are to resolving these issues.

#47 Updated by Stanislav Lomany almost 4 years ago

If you have any remaining questions, please post the list ASAP.

Guys, please consider mouse dragging:
  1. In chrome, when I press mouse within a P2J window, move mouse and release, nothing happens in terms of HTML5 drag and drop.
  2. In chrome, when I press mouse within a window and drag it outside of the window, HTML drag (defined for the HTML document) starts at the border of the window or at the window header and continues on from that point.
  3. In firefox, when I press mouse within a window, HTML drag start if I do a vertical mouse move.

Do you know what defines the boundaries where dragging applies?

I'd also like to know how close you are to resolving these issues.

I did take a look into all of get issues to get familiar with the web part, but haven't made any actual fixes. I'm expecting to fix the listed issues in a week.

#48 Updated by Stanislav Lomany almost 4 years ago

  1. In firefox, when I press mouse within a window, HTML drag start if I do a vertical mouse move.

It is not vertical, just pointer needs to reach some speed. Never mind, the point is that in firefox drag works over a P2J window for some reason. Moreover, I found what I wanted - the code which defines HTML-draggable bounds:

// this must always be set so that the canvas element is removed from the normal layout
// processing of the document
options.style.position = "absolute";

This for some reason doesn't work in firefox.

#49 Updated by Constantin Asofiei almost 4 years ago

Stanislav, if I'm not mistaken Hynek implemented in FWD the resize for legacy 4GL widgets (part of the 'direct manipulation' events).

Why are you looking at the DOM? This resize must happen on the canvas, and driven by the FWD JS driver... the enhanced BROWSE is not using HTML elements, is just a picture on the canvas, right?

Create a simple 4GL test which allows the resize (or move) of a frame via mouse (for example) and see how FWD does that. I suspect you will need something similar.

#50 Updated by Hynek Cihlar almost 4 years ago

Stanislav, you can see how TREELIST column move and resize is implemented in TreeListCaptionItem.

#51 Updated by Greg Shah almost 4 years ago

Also, you should use 4231b because there were many improvements in the drag/drop code in that branch.

#52 Updated by Stanislav Lomany almost 4 years ago

This resize must happen on the canvas, and driven by the FWD JS driver...

Right, it should, but it happens in HTML instead.

#53 Updated by Constantin Asofiei almost 4 years ago

Stanislav Lomany wrote:

This resize must happen on the canvas, and driven by the FWD JS driver...

Right, it should, but it happens in HTML instead.

I don't understand - you want to resize or move a 'virtual' widget (browse row/column) which exists only on the canvas, right?

#54 Updated by Stanislav Lomany almost 4 years ago

Right. I start dragging, then dragging on canvas stops, HTML dragging starts and we have picture like in #3880-44.

#55 Updated by Constantin Asofiei almost 4 years ago

Stanislav Lomany wrote:

Right. I start dragging, then dragging on canvas stops, HTML dragging starts and we have picture like in #3880-44.

Please look at what Hynek mentioned. FWD already supports moving/resizing individual widgets (beside the FWD window).

#56 Updated by Stanislav Lomany almost 4 years ago

Also, you should use 4231b because there were many improvements in the drag/drop code in that branch.

Maybe it is worth merging 3880a into 4231b or trunk?

#57 Updated by Greg Shah almost 4 years ago

If it is safe to do so, put 3880a into 4231b. You've tested your changes in both enhanced and non-enhanced modes?

#58 Updated by Stanislav Lomany almost 4 years ago

If it is safe to do so, put 3880a into 4231b. You've tested your changes in both enhanced and non-enhanced modes?

I think I'll run regression testing before doing it. Isn't 4231b going to trunk any time soon?

#59 Updated by Greg Shah almost 4 years ago

I think we will be trying to clean it up and get it into trunk over the next two weeks. We have known issues for this branch to clear in at least one customer's large GUI application. I don't think we've tested the ChUI regression in this branch in the last month so we need to get through any issues there. Then we will do wider testing.

But it makes sense to get 3880a clean before putting it in 4231b. I prefer not to have a rebase in 4231b, so that is why I want you to merge there.

#60 Updated by Stanislav Lomany almost 4 years ago

Also, you should use 4231b because there were many improvements in the drag/drop code in that branch.

Yes, in chrome you now don't lose drag if you drag outside the window. For firefox everything stays the same.

#61 Updated by Stanislav Lomany almost 4 years ago

To make sure we're on the same page. That's how drag works for me in the latest firefox: https://youtu.be/4XY0TKhKFRY Can you confirm the issue? Chrome is perfectly fine.

#62 Updated by Hynek Cihlar almost 4 years ago

Stanislav Lomany wrote:

Also, you should use 4231b because there were many improvements in the drag/drop code in that branch.

Yes, in chrome you now don't lose drag if you drag outside the window. For firefox everything stays the same.

Perhaps this is due to the differences in how the event default behavior is prevented in Chrome and Firefox. Check the mouse event handlers how this is done there. Also see https://developer.mozilla.org/en-US/docs/Web/API/Event/preventDefault.

#63 Updated by Stanislav Lomany almost 4 years ago

OK, HTML body was explicitly defined draggable for some reason. So, on every drag in firefox we were dragging body element and the "artefact" in the form of gray line in the picture from #3880-44 is a preview picture of the body and the line is an image of the bottom P2J taskbar.

<body class="claro" draggable="true" style="font-family:Arial,Helvetica,Verdana,serif;font-size:${font.size}px;">

Removing draggable attribute fixes the drag problem. Let me know if you have any concerns.

#64 Updated by Greg Shah almost 4 years ago

Sergey/Hynek/Constantin: Please review.

Sergey: the important parts are from #3880-44 to #3880-63.

#65 Updated by Hynek Cihlar almost 4 years ago

Stanislav Lomany wrote:

OK, HTML body was explicitly defined draggable for some reason. So, on every drag in firefox we were dragging body element and the "artefact" in the form of gray line in the picture from #3880-44 is a preview picture of the body and the line is an image of the bottom P2J taskbar.

[...]

Removing draggable attribute fixes the drag problem. Let me know if you have any concerns.

Stanislav, please search in bzr blame/log who and why was this attribute added. This could be needed for some of the native features, like file drag & drop.

#66 Updated by Constantin Asofiei almost 4 years ago

This looks like is Eugenie's, from trunk rev 11770:

ca@xuxa:/working/gcd.files/gcd.bzr/p2j$ bzr diff -r 11169..11170 src/com/goldencode/p2j/ui/client/driver/web/index.html
=== modified file 'src/com/goldencode/p2j/ui/client/driver/web/index.html'
--- src/com/goldencode/p2j/ui/client/driver/web/index.html      2017-06-20 06:10:28 +0000
+++ src/com/goldencode/p2j/ui/client/driver/web/index.html      2017-08-25 02:19:38 +0000
@@ -170,7 +170,7 @@
       });
       </script>
    </head>
-   <body class="claro" style="font-family:Arial,Helvetica,Verdana,serif;font-size:${font.size}px;">
+   <body class="claro" draggable="true" style="font-family:Arial,Helvetica,Verdana,serif;font-size:${font.size}px;">
       <div id="cont" style="margin-left:auto;margin-right:auto;">

          <noscript>Your browser does not support JavaScript!</noscript>

ca@xuxa:/working/gcd.files/gcd.bzr/p2j$ bzr log -r 11170
------------------------------------------------------------
revno: 11170 [merge]
committer: Eugenie V. Lyzenko <evl@goldencode.com>
branch nick: trunk
timestamp: Wed 2017-09-27 15:45:05 +0300                                                                                                                                                                                                                                           
message:                                                                                                                                                                                                                                                                           
  Direct manipulation implementation part 2, drag and drop support added (refs #1834b).                                                                                                                                                                                            
------------------------------------------------------------                                                                                                                                                                                                                       
Use --include-merged or -n0 to see merged revisions.                                                                                                                                                                                                                               

Eugenie: do you recall why draggable HTML body was required for drag and drop?

Hynek: can the body draggable setting affect the other cases of movable/resizeable widgets? I'm a little confused, if this issue is specific to what Stanislav works on or is a global problem for movable/resizeable widgets.

#67 Updated by Hynek Cihlar almost 4 years ago

Constantin Asofiei wrote:

Hynek: can the body draggable setting affect the other cases of movable/resizeable widgets? I'm a little confused, if this issue is specific to what Stanislav works on or is a global problem for movable/resizeable widgets.

AFAIK direct manipulation is fully implemented in the canvas and no native HTML support is required. So the draggable attribute should not be needed.

#68 Updated by Hynek Cihlar almost 4 years ago

Hynek Cihlar wrote:

Constantin Asofiei wrote:

Hynek: can the body draggable setting affect the other cases of movable/resizeable widgets? I'm a little confused, if this issue is specific to what Stanislav works on or is a global problem for movable/resizeable widgets.

AFAIK direct manipulation is fully implemented in the canvas and no native HTML support is required. So the draggable attribute should not be needed.

This also applies to the other widget resize/drag features.

#69 Updated by Greg Shah almost 4 years ago

What about drag and drop between windows (across canvas instances)?

#70 Updated by Constantin Asofiei almost 4 years ago

I think the problem is global; I'm having issues with dragging a button in FWD, on both Chrome and Firefox. See this test:

def button btn.

form btn with frame f1 size 20 by 20.
btn:movable in frame f1 = true.
update btn with frame f1.

In Chrome, first drag works, after that it no longer works.
In Firefox, the first attempt at moving the button is not working at all.

#71 Updated by Eugenie Lyzenko almost 4 years ago

Constantin Asofiei wrote:

This looks like is Eugenie's, from trunk rev 11770:
[...]

Eugenie: do you recall why draggable HTML body was required for drag and drop?

Either to support DnD from OS to browser and back or to use direct manipulation widget feature inside browser(for window widget too).

#72 Updated by Hynek Cihlar almost 4 years ago

Greg Shah wrote:

What about drag and drop between windows (across canvas instances)?

I'm not aware of any use case where direct manipulation would involve widget dragging across windows.

#73 Updated by Hynek Cihlar almost 4 years ago

Eugenie Lyzenko wrote:

Constantin Asofiei wrote:

This looks like is Eugenie's, from trunk rev 11770:

Eugenie: do you recall why draggable HTML body was required for drag and drop?

Either to support DnD from OS to browser and back or to use direct manipulation widget feature inside browser(for window widget too).

But isn't the purpose of draggable attribute to allow dragging of DOM elements? Direct manipulation happens fully in the context of the canvas image buffer, so draggable shouldn't apply there, right?

Dropping files from system to browser doesn't seem to require draggable attribute. See https://jsfiddle.net/user2314737/mnfztctt/.

#74 Updated by Stanislav Lomany almost 4 years ago

Guys, what FWD feature supports dragging files from OS so I can check that it works?

#75 Updated by Stanislav Lomany almost 4 years ago

In Chrome, first drag works, after that it no longer works.

I can confirm the issue. After the dragging fix, it behaves in the same way for firefox too. Should I take a look into it?

#76 Updated by Sergey Ivanovskiy almost 4 years ago

Constantin Asofiei wrote:

I think the problem is global; I'm having issues with dragging a button in FWD, on both Chrome and Firefox. See this test:
[...]
In Chrome, first drag works, after that it no longer works.
In Firefox, the first attempt at moving the button is not working at all.

Yes, I reproduced this testcase with 4231b. Does it make sense to create new issue. For the swing client this test works properly. It seems that this regression is not related to #3880.

#77 Updated by Sergey Ivanovskiy almost 4 years ago

Constantin Asofiei wrote:

I think the problem is global; I'm having issues with dragging a button in FWD, on both Chrome and Firefox. See this test:
[...]
In Chrome, first drag works, after that it no longer works.

I tested with 4231b in Chrome and found that the dragging operations for this button can be restarted after the button's owner window has been collapsed(minimized) and restored.

#78 Updated by Stanislav Lomany almost 4 years ago

It is interesting that column resize is implemented in BrowseColumnGuiImpl.mouseMoved (it works), while row resize was moved to MouseDirectManipulation (it doesn't work). I don't see conceptual difference between two resize types, but they are in different places.

I have a question about AbstractWidget.setMousePointer. Simplified code:

// pointer is set for the widget at JS side
ThinClient.propagate(topLevelWindow().getId().asInt(), getId().asInt(), ptr);

...   
if (recentlyHovered != null && recentlyHovered.windowId() == wnd.getId().asInt())
{
  setCursor(recentlyHovered.getEffectiveMousePointer());
}
else
{
  setCursor(window.getEffectiveMousePointer());

}

recentlyHovered is null so what happens is resize pointer is set on JS side with propagate and then immediately reset to the window default pointer. What is wrong in this chain? Should I use TC.pushHover to set the current widget as recentlyHovered?

#79 Updated by Sergey Ivanovskiy almost 4 years ago

Stanislav Lomany wrote:

It is interesting that column resize is implemented in BrowseColumnGuiImpl.mouseMoved (it works), while row resize was moved to MouseDirectManipulation (it doesn't work). I don't see conceptual difference between two resize types, but they are in different places.

I have a question about AbstractWidget.setMousePointer. Simplified code:
[...]
recentlyHovered is null so what happens is resize pointer is set on JS side with propagate and then immediately reset to the window default pointer. What is wrong in this chain? Should I use TC.pushHover to set the current widget as recentlyHovered?

It seems that MouseHoverAction is responsible for changing mouse pointer and it uses directly TC.pushHover. It seems that BrowseColumnGuiImpl.mouseActions should have MouseEvt.MOUSE_HOVERABLE. In this case the hoverable column regions will be known to JS client.

#80 Updated by Stanislav Lomany almost 4 years ago

Sergey, it doesn't work. Should I explicitly create MouseHoverAction for browse column? How can I specify a hoverable area inside the widget using this method?

#81 Updated by Stanislav Lomany almost 4 years ago

Actually, area is defined in MouseDirectManipulation, but it doesn't work because of the problem noted above. So should I add MouseHoverAction for this part to work?

#82 Updated by Sergey Ivanovskiy almost 4 years ago

Stanislav Lomany wrote:

Sergey, it doesn't work. Should I explicitly create MouseHoverAction for browse column? How can I specify a hoverable area inside the widget using this method?

It seems that hoverable area is defined by widget.getActualBounds(). I searched all usages of getActualBounds. AbstractWidget has the following method that creates MouseHoverAction. It needs to debug the code. What test case can help to reproduce these issues?

   private void _setEnabled(boolean enabled)
   {
      this.enabled = enabled;
      if (config() != null)
      {
         config().enabled = enabled;
      }
      else
      {
         return;
      }

      if (!supportsCustomMousePointer() || 
            ((this instanceof Frame) && !config().realized))
      {
         return;
      }

      if (!enabled && mouseHoverAction == null)
      {
         return;
      }

      if (parent() != null || this instanceof Frame)
      {
         if (mouseHoverAction == null)
         {
            mouseHoverAction = createMouseHoverAction(); 
         }

         mouseHoverAction.gd().selectWindow(topLevelWindow().getId().asInt());

         if (enabled)
         {
            mouseHoverAction.register();
         }
         else
         {
            mouseHoverAction.deregister();
         }

         mouseHoverAction.gd().releaseWindow();
      }
   }

#83 Updated by Stanislav Lomany almost 4 years ago

What test case can help to reproduce these issues?

Is is attached: if you try to resize a row, you'll notice that the resize cursor is missing in a browser (but resize itself works).
BTW I couldn't find when/if MouseHoverAction.mouseEntered (which is supposed to push recentlyHovered) got called.

#84 Updated by Sergey Ivanovskiy almost 4 years ago

Thanks, I tested browse-simple.p with 4231b and Firefox 76.0.1 on Ubuntu. It seems that the resizing column works partly in this branch, the resize cursor is displayed but only it has left to right direction. It needs an extra click of the left mouse button after dragging for resizing action has been finished. It can be that some recent changes in 4231b can affect this behaviour. Resizing rows doesn't work . It seems that the cursor area for changing cursor styles is so that the resizing cursor can't be stable, it is changed to the default cursor when the mouse is clicked and moved. I think we should list all issues and create a new bug issue related to resizing and work on these issues because it seems that they are not related to this task.

#85 Updated by Hynek Cihlar almost 4 years ago

Stanislav Lomany wrote:

Sergey, it doesn't work. Should I explicitly create MouseHoverAction for browse column? How can I specify a hoverable area inside the widget using this method?

MouseHoverAction is created in AbstractWidget._setEnabled(). Also it must be registered with the driver. This depends on the enabled state of your resizable widget. If it gets registered, find out if the mouse enter and mouse leave are sent for your resizable widget down to Java client. These events, or they handlers in MouseHoverAction, are responsible for calling TC.pushHover and TC.popHover. This is where the recentlyHoevered gets assigned.

Disclaimer, I'm not the author of the hover code and the advice is based on static code analysis :-).

#86 Updated by Greg Shah almost 4 years ago

and create a new bug issue related to resizing and work on these issues because it seems that they are not related to this task.

Actually, it is part of this task because without these changes the current implementation of enhanced browse is broken. We previously deferred this work and this #3880 task is where we fix this. We've already spent 2-3 weeks on the drag/drop/mouse processing issues. I'm hoping that we can fix these issues and close this off ASAP.

#87 Updated by Stanislav Lomany almost 4 years ago

Thanks, I tested browse-simple.p with 4231b and Firefox 76.0.1 on Ubuntu. It seems that the resizing column works partly in this branch, the resize cursor is displayed but only it has left to right direction.

That's a normal behavior.

It needs an extra click of the left mouse button after dragging for resizing action has been finished.

I haven't met this issue. I suppose that's because my changes that fix firefox in general are not committed yet.

#88 Updated by Stanislav Lomany almost 4 years ago

Some thoughts:
  1. MouseHoverAction is registered using GuiWebDriver.registerHoverableWidget on the JS side and doesn't get registered in MouseHandler.WorkArea.actions. Use of MouseHoverAction is to set widget-specific cursor without round-trip to Java side.
  2. I don't understand when MouseHoverAction.mouseEntered which sets TC.recentlyHovered got called. It is theoretically can be called from MouseHandler.processWidgetActions, but it doesn't get registered in the set of actions, as I noted above. And even if it was registered, invocation order relatively to MouseDirectManipulation would not be guaranteed.
  3. So, when MouseDirectManipulation fires, it doesn't find TC.recentlyHovered.
  4. Overall, this pointer calling construction seem flimsy to me. So I've added this code to MouseDirectManipulation:
    if (<row resize condition>)
    {
       gd.setCursor(ptrNew); // added
    }
    widget.setMousePointer(ptrNew); // should be calling gd.setCursor(ptrNew) but it doesn't because of the reasons noted above
    

I'm going to create a separate issue for a widget not able to be moved for the second time.

One question: what 4GL functionality contains file drop to browser so I could test is and close this part of web issues?

#89 Updated by Hynek Cihlar almost 4 years ago

Stanislav Lomany wrote:

Some thoughts:
  1. MouseHoverAction is registered using GuiWebDriver.registerHoverableWidget on the JS side and doesn't get registered in MouseHandler.WorkArea.actions. Use of MouseHoverAction is to set widget-specific cursor without round-trip to Java side.

I may be interpreting the code wrong, but MouseHoverAction.register up calls JS client which makes it to create MouseHoverable object. MouseHoverable handles mouse enter and mouse exit events. The respective event handlers send socket messages to Java client. Unless there is another use case where MouseHoverAction sets widget-specific cursor without round-trip to Java side.

  1. So, when MouseDirectManipulation fires, it doesn't find TC.recentlyHovered.

MouseDirectManipulation AFAIK only handles the specific use cases for direct widget manipulation, i.e. when you set the MOVABLE and RESIZABLE attributes. So it should not be of interest for your case.

One question: what 4GL functionality contains file drop to browser so I could test is and close this part of web issues?

See the DROP-TARGET attribute, https://documentation.progress.com/output/ua/OpenEdge_latest/index.html#page/dvref%2Fdrop-target-attribute.html.

#90 Updated by Stanislav Lomany almost 4 years ago

Guys, all (most?) of the fonts look bigger in a browser compared to a standalone app. The lager the font, the bigger the difference, some fonts "behave" better than the others. When a user changes browse font, row height is calculated on the size of the font in the standalone app, later user can adjust row height. Can we have the same looking fonts in browser and standalone app?

#91 Updated by Greg Shah almost 4 years ago

There is no way to get exactly the same results as the 4GL, even with the same exact fonts being used. We employ a range of tricks to get close.

  • We pick replacement fonts where we attempt to match the look and metrics of the results as closely as possible.
  • We capture the font metrics from the legacy fonts on a windows platform and use those at runtime.
  • We capture the text metrics from the specific UI labels, titles etc on a windows platform and use those at runtime.
  • At runtime, we attempt to scale the rendered text to fit inside the legacy metrics, especially width.
  • And more...

Your found difference is especially large. Normally we don't see a difference that big. The use of multiple W chars is probably "magnifying" the problem. I wonder if it would be better if you had captured metrics for that "WWWWWWWW" string.

#92 Updated by Stanislav Lomany almost 4 years ago

The upper left corner of the browse has no purpose, in enhanced mode it will be drawn as a gear icon instead of the 3D placeholder that exists there now

Greg, I'm not sure what placeholder are you talking about. I can draw a placeholder in a corner (but it may overlap column header or the first row, if there are no headers and no browse title).

When "change layout" mode is active, should we use the same gear to call what is now right-click menu? I'm asking because we have column-specific options which are active when we right-click on a column. So in this case we should provide an in-menu list of columns to alter to to draw gears on each column.

#93 Updated by Stanislav Lomany almost 4 years ago

Your found difference is especially large. Normally we don't see a difference that big. The use of multiple W chars is probably "magnifying" the problem. I wonder if it would be better if you had captured metrics for that "WWWWWWWW" string.

Sorry, so what my plan should be for this problem (if any)?

#94 Updated by Greg Shah almost 4 years ago

The upper left corner of the browse has no purpose, in enhanced mode it will be drawn as a gear icon instead of the 3D placeholder that exists there now

Greg, I'm not sure what placeholder are you talking about. I can draw a placeholder in a corner (but it may overlap column header or the first row, if there are no headers and no browse title).

Please post examples of the possible states that we can see for the the browse (with different options active). I'm especially interested in the top left corner, but we can also consider each of the other corners if there is a better option. Then we can discuss.

When "change layout" mode is active, should we use the same gear to call what is now right-click menu? I'm asking because we have column-specific options which are active when we right-click on a column. So in this case we should provide an in-menu list of columns to alter to to draw gears on each column.

Yes, this is the idea to eliminate our reliance on the context menu.

Your found difference is especially large. Normally we don't see a difference that big. The use of multiple W chars is probably "magnifying" the problem. I wonder if it would be better if you had captured metrics for that "WWWWWWWW" string.

Sorry, so what my plan should be for this problem (if any)?

Are you talking about item 3.2 from #3880-4?

#95 Updated by Stanislav Lomany almost 4 years ago

Are you talking about item 3.2 from #3880-4?

Yes.

#96 Updated by Greg Shah almost 4 years ago

Stanislav Lomany wrote:

Are you talking about item 3.2 from #3880-4?

Yes.

This is a user override, so we can modify the row height because there is no compatibility requirement. The core thing here is to make sure that the row height is changed to fully display any user-selected font. I suppose that means you must determine what the minimum safe row height will be for that font and implement it.

#97 Updated by Stanislav Lomany almost 4 years ago

I suppose that means you must determine what the minimum safe row height will be for that font and implement it.

That means in the standalone app the rows will tend to be higher than they need to be. Is it OK?

#98 Updated by Greg Shah almost 4 years ago

Stanislav Lomany wrote:

I suppose that means you must determine what the minimum safe row height will be for that font and implement it.

That means in the standalone app the rows will tend to be higher than they need to be. Is it OK?

No. Please explain why this would be the case.

#99 Updated by Stanislav Lomany almost 4 years ago

User changes font in web version, we set row height according to web font metrics, everything is looking nice. If the same browse is opened in standalone version, where fonts tend to be smaller, it will be a smaller text in a larger row.

#100 Updated by Greg Shah almost 4 years ago

Why is the row height static? It should be calculated based on whatever is being used at the time. I guess this means when the font is changed and when the browse is first displayed with a custom font choice. So the value can be different for different clients because they handle the font rendering differently.

#101 Updated by Stanislav Lomany almost 4 years ago

Please post examples of the possible states that we can see for the the browse (with different options active).

#102 Updated by Greg Shah almost 4 years ago

  • Is there always a vertical scrollbar?
  • Aren't there cases where there are row markers?
  • Please show the same cases which also have the horizontal scrollbars.

#103 Updated by Stanislav Lomany almost 4 years ago

  • Is there always a vertical scrollbar?

It can be turned off.

  • Aren't there cases where there are row markers?

They are visible in editable mode (they can be turned off).

  • Please show the same cases which also have the horizontal scrollbars.

#104 Updated by Greg Shah almost 4 years ago

Is it possible to have a horizontal scrollbar without a vertical scrollbar?

#105 Updated by Stanislav Lomany almost 4 years ago

Yes.

#106 Updated by Greg Shah almost 4 years ago

Please show an example.

#107 Updated by Stanislav Lomany almost 4 years ago

#108 Updated by Greg Shah almost 4 years ago

We will use the bottom right corner of the browse. In enhanced mode (simple boolean flag), we will always provide the gear icon in the bottom right.

Here are the cases:

  1. If there are both horizontal AND vertical scrollbars, then there is a natural "dead spot" in that corner. Draw the gear icon in that dead spot.
  2. If there is a horizontal scrollbar AND NO vertical scrollbar, then we will create the same "dead spot" in that corner by shifting the end of the horizontal scrollbar to the left. Draw the gear icon in that dead spot.
  3. If there is NO horizontal scrollbar, we will replace the bottommost row of the browse data with a "tray" that can be drawn with the same color as the "dead spot". This will leave plenty of space and we can still draw the gear icon in the same bottom right location.

Can you see any case which this does not cover?

#109 Updated by Stanislav Lomany almost 4 years ago

Can you see any case which this does not cover?

If there is a vertical scrollbar and NO horizontal scrollbar, we should create a dead spot by shifting the end of the vertical scrollbar to the top.

#110 Updated by Greg Shah almost 4 years ago

Stanislav Lomany wrote:

Can you see any case which this does not cover?

If there is a vertical scrollbar and NO horizontal scrollbar, we should create a dead spot by shifting the end of the vertical scrollbar to the top.

To the top of the last row? I thought about this but then the last row will look strange. I thought that this case would be part of case 3, where we create a "tray" by replacing the last row. Of course, when creating the "tray", any vertical scrollbar would only be drawing up to the visible rows (which would be row 4 in the examples above).

#111 Updated by Stanislav Lomany almost 4 years ago

I mean like this. We can make a tray, but why lose a row? In this case we'll be losing a row only if there are no scroll bars at all (which shouldn't be a typical case).

#112 Updated by Greg Shah almost 4 years ago

Code Review Task Branch 3880a Revisions 11347 through 11353

1. I really like the refactoring of the TC dialog processing. It is much better in your changes.

2. Saving TC instance in ClientBrowseConfigManager is not safe. There are cases in which the JVM and client stay around but the TC (and all the other client daemon processes) are reset/replaced. This occurs on an unhandled STOP condition where the client re-executes the startup procedure. Please see TC.initializePrep().

3. The static instance of enhancedSaveTargets in BrowseGuiImpl is probably OK. The JVM for a FWD client can't be kept around across OS level logins.

4. The KW_ALIGN added to progress.g should have 0 set for the abbreviation. Abbreviations are a horrible thing. I don't want to add new ones in keywords that we control.

5. The Browse.Alignment enum should be in the ui package and not the ui/client/ since it is used on the server too.

6. In Browse.java, the 164 history entry should just be a sub-entry of 163.

#113 Updated by Stanislav Lomany almost 4 years ago

Why is the row height static? It should be calculated based on whatever is being used at the time.

Currently, there's no "auto" row height option. Row height is saved along with other browse parameters as a part of enhanced configuration. I can set row height to "auto" by default. Specific row height value will be stored if user explicitly resizes a row. Also we'll need to add row height reset option to the right-click menu. Is that the way to go?

#114 Updated by Greg Shah almost 4 years ago

I mean like this. We can make a tray, but why lose a row? In this case we'll be losing a row only if there are no scroll bars at all (which shouldn't be a typical case).

Agreed. It is better.

#115 Updated by Greg Shah almost 4 years ago

Stanislav Lomany wrote:

Why is the row height static? It should be calculated based on whatever is being used at the time.

Currently, there's no "auto" row height option. Row height is saved along with other browse parameters as a part of enhanced configuration. I can set row height to "auto" by default. Specific row height value will be stored if user explicitly resizes a row. Also we'll need to add row height reset option to the right-click menu. Is that the way to go?

Yes.

#116 Updated by Stanislav Lomany almost 4 years ago

Fix for web issues (web column/row resize) committed into 4231b as rev 11603.

BTW there's one more minor web issue: cursor switches back to default as soon as row/column resizing starts by moving the mouse after the mouse button is down.

#117 Updated by Stanislav Lomany almost 4 years ago

3880a has been merged into 4231b as rev 11604. 3880a has been archived.

#118 Updated by Stanislav Lomany almost 4 years ago

Regarding the font size: I found that actually may be it's not the case that a web font is larger, it's the case that standalone app font is smaller. Specifically, I found that the font drawn in a font preview window using buffered image is larger than the same font applied to browse. Do you have an idea what we're are doing about Graphics2D or something else that could cause this difference? Web version doesn't have this difference.
BTW I found that in widgets code, including browse, font height is handled like it is in pixels, which implies resolution of 72 dpi and therefore 1 point = 1 pixel.

#119 Updated by Greg Shah almost 4 years ago

Do you have an idea what we're are doing about Graphics2D or something else that could cause this difference? Web version doesn't have this difference.

I don't know the answer.

Do we have accurate metrics that differ between Swing and web? If so, can we adjust our sizing to honor the different metrics with correct results? In other words, can we size things to ensure that the text is fully visible?

#120 Updated by Stanislav Lomany almost 4 years ago

Do we have accurate metrics that differ between Swing and web?

The metrics are the same, actual text size differs. I got lucky to find that the font is correct when it is applied to the browse initially rather than being selected from the font selection window, so probably I missed some scaling along the way (like GuiDriver.scaleFont).

#121 Updated by Greg Shah almost 4 years ago

Are the metrics calculated before the scaling? The thing I don't understand is that the Swing browse draws everything in a way that matches the scaled font. If scaling only happens for Swing, then why are the metrics for web the same (and wrong)?

#122 Updated by Stanislav Lomany almost 4 years ago

If scaling only happens for Swing, then why are the metrics for web the same (and wrong)?

It's not quite that way. These elements have correct font size:
  1. web browse
  2. web font picker
  3. swing font picker
Swing browse doesn't have correct font size. Unless it was initialized from a user-specific storage which keeps FontDetails as an object.
Here is the sequence that leads to it:
  1. When font is applied from the font chooser, it has no legacy metrics.
  2. The font is NOT scaled. Actual font metrics are used as legacy metrics.
  3. FontDetails is saved with legacy metrics.
  4. Server is restarted, FontDetails is loaded with legacy metrics.
  5. Because there are legacy metrics, font IS scaled.
This is possible due to AbstractGuiDriver.scaleFont logic, which is:
  1. No legacy metric? Do nothing.
  2. Compare actual font metrics and legacy metrics. Same? Do nothing.
  3. Multiply font size by 4 / 3 (aka 96 / 72 aka getScreenResolution() / 72). So when I say "scaled", I mean it is multiplied by 4 / 3.

In reality that means that depending on presence of "legacy" metrics a font size is multiplied by 4 / 3 or not.
A font in font chooser is always scaled:

g.setFont(new Font(strFace, style, size * 4 / 3)); // convert point to pixel size

I'm not sure how to better address this issue.

#123 Updated by Greg Shah almost 4 years ago

The font chooser case is not very important. If that was not working, it would be less of a concern.

The big concern here is that the web gives different results from Swing. Based on your notes, I still don't understand why this is the case. If you know why, then I need you to explain it differently because I don't see it at the moment.

#124 Updated by Stanislav Lomany almost 4 years ago

The big concern here is that the web gives different results from Swing. Based on your notes, I still don't understand why this is the case.

To draw in web, we register a font at JS side with the target size in points and use it by referencing its numerical id.
To draw in swing, we have to scale a font by 4/3 to achieve the font of the same size. And this scaling fails for a subset of cases where font picker has been used.
I guess I have to register a font from the font picker is a special way which will affect only swing.

#125 Updated by Hynek Cihlar almost 4 years ago

Stanislav Lomany wrote:

The big concern here is that the web gives different results from Swing. Based on your notes, I still don't understand why this is the case.

To draw in web, we register a font at JS side with the target size in points and use it by referencing its numerical id.
To draw in swing, we have to scale a font by 4/3 to achieve the font of the same size. And this scaling fails for a subset of cases where font picker has been used.
I guess I have to register a font from the font picker is a special way which will affect only swing.

The scale factor should be different for Web as well as for Swing client. So why does the font chooser always uses 4/3?

#126 Updated by Stanislav Lomany almost 4 years ago

The scale factor should be different for Web as well as for Swing client. So why does the font chooser always uses 4/3?

If you believe in forums Toolkit.getDefaultToolkit().getScreenResolution() always returns 96 for screen DPI unless some environment parameter is set, so it's always 4/3 for swing and web and I suppose someone just hardcoded it. But yes, it should be replaced.

#127 Updated by Greg Shah almost 4 years ago

What is the "simple" (single boolean flag) directory configuration for enabling enhanced mode?

#128 Updated by Stanislav Lomany almost 4 years ago

Put this under /server/default/ or /server/<server_name>/:

<node class="container" name="enhancedBrowse">
   <node class="container" name="desktop">
      <node class="boolean" name="enableAll">
         <node-attribute name="value" value="TRUE"/>
      </node>
   </node>
   <node class="container" name="embedded">
      <node class="boolean" name="enableAll">
         <node-attribute name="value" value="TRUE"/>
      </node>
   </node>
</node> 

#129 Updated by Eugenie Lyzenko almost 4 years ago

In Web client the gui/driver/web/GuiWebDriver.java uses specific X scaling factor for every font in use. This should make the difference for font usage in Swing and Web. For vertical dimension there is no difference as far as I know. The web driver scaling factor considers the font type and style. Also it does special processing for small fonts when the difference is bigger than for medium and large font size.

As general consideration in Web client case we have less control for how the text is rendered. We can ask the JS to do something, the final decision will be up to JS side.

#130 Updated by Stanislav Lomany almost 4 years ago

An enhanced browse configuration cannot be loaded from a web storage. I have to fix it before any other issue. If anyone finds that familiar, let me know.

com.fasterxml.jackson.core.JsonParseException: Illegal character ((CTRL-CHAR, code 0)): only regular white space (\r, \n, \t) is allowed between tokens
 at [Source: ; line: 1, column: 2]
    at com.fasterxml.jackson.core.JsonParser._constructError(JsonParser.java:1702)
    at com.fasterxml.jackson.core.base.ParserMinimalBase._reportError(ParserMinimalBase.java:558)
    at com.fasterxml.jackson.core.base.ParserMinimalBase._throwInvalidSpace(ParserMinimalBase.java:509)
    at com.fasterxml.jackson.core.json.ReaderBasedJsonParser._skipWSOrEnd(ReaderBasedJsonParser.java:2364)
    at com.fasterxml.jackson.core.json.ReaderBasedJsonParser.nextToken(ReaderBasedJsonParser.java:644)
    at com.fasterxml.jackson.databind.ObjectMapper._initForReading(ObjectMapper.java:3834)
    at com.fasterxml.jackson.databind.ObjectMapper._readMapAndClose(ObjectMapper.java:3783)
    at com.fasterxml.jackson.databind.ObjectMapper.readValue(ObjectMapper.java:2842)
    at com.goldencode.p2j.ui.client.gui.driver.web.WebClientKeyValueStorage.getChunk(WebClientKeyValueStorage.java:283)
    at com.goldencode.p2j.ui.client.gui.driver.web.WebClientKeyValueStorage.getFirstChunk(WebClientKeyValueStorage.java:194)
    at com.goldencode.p2j.ui.client.UserKeyValueStorage.getString(UserKeyValueStorage.java:180)
    at com.goldencode.p2j.ui.KeyValueStorageAdapter.parseJSON(KeyValueStorageAdapter.java:244)
    at com.goldencode.p2j.ui.KeyValueStorageAdapter.getObject(KeyValueStorageAdapter.java:206)
    at com.goldencode.p2j.ui.chui.ThinClient.getObject(ThinClient.java:24116)
    at com.goldencode.p2j.ui.client.ClientBrowseConfigManager.getEnhancedConfig(ClientBrowseConfigManager.java:178)
    at com.goldencode.p2j.ui.client.ClientBrowseConfigManager.getClientEnhancedConfigs(ClientBrowseConfigManager.java:161)
    at com.goldencode.p2j.ui.chui.ThinClient.getClientEnhancedConfigs(ThinClient.java:10545)
    at com.goldencode.p2j.ui.ClientExportsMethodAccess.invoke(Unknown Source)
    at com.goldencode.p2j.util.MethodInvoker.invoke(MethodInvoker.java:156)
    at com.goldencode.p2j.net.Dispatcher.processInbound(Dispatcher.java:755)
    at com.goldencode.p2j.net.Conversation.block(Conversation.java:412)
    at com.goldencode.p2j.net.Conversation.waitMessage(Conversation.java:348)
    at com.goldencode.p2j.net.Queue.transactImpl(Queue.java:1201)
    at com.goldencode.p2j.net.Queue.transact(Queue.java:672)
    at com.goldencode.p2j.net.BaseSession.transact(BaseSession.java:271)
    at com.goldencode.p2j.net.HighLevelObject.transact(HighLevelObject.java:211)
    at com.goldencode.p2j.net.RemoteObject$RemoteAccess.invokeCore(RemoteObject.java:1473)
    at com.goldencode.p2j.net.InvocationStub.invoke(InvocationStub.java:145)
    at com.sun.proxy.$Proxy11.standardEntry(Unknown Source)
    at com.goldencode.p2j.main.ClientCore.start(ClientCore.java:380)
    at com.goldencode.p2j.main.ClientCore.start(ClientCore.java:167)
    at com.goldencode.p2j.main.ClientDriver.start(ClientDriver.java:250)
    at com.goldencode.p2j.main.CommonDriver.process(CommonDriver.java:444)
    at com.goldencode.p2j.main.ClientDriver.process(ClientDriver.java:144)
    at com.goldencode.p2j.main.ClientDriver.main(ClientDriver.java:313)

#131 Updated by Sergey Ivanovskiy almost 4 years ago

I didn't encounter this exception. Could you provide a testcase to reproduce? At the time of my changes the enhanced browse configuration was serialized via the web client. Does it make sense to clean the browser cache in order to understand what is the issue?

#132 Updated by Sergey Ivanovskiy almost 4 years ago

I checked the Hotel Gui with the web client for Firefox. It seems that the enhanced browse configuration is stored. It uses key with null values but I didn't find another exceptions in the server log.

#133 Updated by Sergey Ivanovskiy almost 4 years ago

Also I tried to save the browse configuration with blue background color for the particular user and restore it after logout and this test passed. Please look at the localStorage there are two stored configurations now.

#134 Updated by Stanislav Lomany almost 4 years ago

Sergey, does configurations survive server restart? It just crossed my mind that a dynamic port is assigned every time, and it may be considered as another site. If that's true then how an enhanced config is supposed to be stored on localhost?

#135 Updated by Stanislav Lomany almost 4 years ago

Regarding exception. It is suppressed in WebClientKeyValueStorage.getChunk:

catch (IOException e)
{
   lastRetrievedChunk = null;
   e.printStackTrace(); <-- added
}

Looks Java side receives malformed message with a string of 0-chars.

#136 Updated by Sergey Ivanovskiy almost 4 years ago

Stanislav Lomany wrote:

Sergey, does configurations survive server restart?

Yes, after the server has been restarted the configuration was not retrieved but it is not because of the different port number. I observed this key enhancedBrowse/desktop/bogus/guests-frame.w/staysBrowse/expr 1("character", "x(40)", "");expr 2("integer", ?, "");roomType.description;stay.endDate;stay.numGuests;stay.price;stay.roomNum;stay.startDate for the enchanced browse config of the hotel/hotel user and me/OS user. Does it relate to the key value? Does this key have the same value after restarting? If the server is not restarted, then the configuration is restored.

It just crossed my mind that a dynamic port is assigned every time, and it may be considered as another site. If that's true then how an enhanced config is supposed to be stored on localhost?

Yes, the local storage depends on the web page path.

#137 Updated by Stanislav Lomany almost 4 years ago

Yes, after the server has been restarted the configuration was not retrieved but it is not because of the different port number. I observed this key enhancedBrowse/desktop/bogus/guests-frame.w/staysBrowse/expr 1("character", "x(40)", "");expr 2("integer", ?, "");roomType.description;stay.endDate;stay.numGuests;stay.price;stay.roomNum;stay.startDate for the enchanced browse config of the hotel/hotel user and me/OS user. Does it relate to the key value?

The key looks normal. Well, except that the actual user name is "bogus", but I don't know what security model is implemented and I don't think that's relevant to the problem.

Does this key have the same value after restarting?

It should be the same.

#138 Updated by Sergey Ivanovskiy almost 4 years ago

In this case it seems that the browse configuration doesn't read after the user is logged out and it is saved on the server side correct?

#139 Updated by Sergey Ivanovskiy almost 4 years ago

It is not obvious for me that the issue is the serialization/deserealization from the web localStorage. Planning to debug what was the issue.

#140 Updated by Stanislav Lomany almost 4 years ago

User configurations can be "for all users" and user-specific. "For all users" are saved in the directory. User-specific - in browser local storage (and that's it).

#141 Updated by Stanislav Lomany almost 4 years ago

Planning to debug what was the issue.

I think I found the bug:

writeStringBinaryMessage(msg, offset, value);
...
function writeStringBinaryMessage(message, text, offset)

#142 Updated by Sergey Ivanovskiy almost 4 years ago

The first I checked that the localStorage has the same key and value after the server has been restarted. I observed this key/value pair

enhancedBrowse/desktop/bogus/guests-frame.w/staysBrowse/expr 1("character", "x(40)", "");expr 2("integer", ?, "");roomType.description;stay.endDate;stay.numGuests;stay.price;stay.roomNum;stay.startDate

"{\"value\":\"{\\\"ehKeyProcName\\\":\\\"guests-frame.w\\\",\\\"ehKeyBrowseName\\\":\\\"staysBrowse\\\",\\\"userName\\\":\\\"bogus\\\",\\\"ehBgColor\\\":{\\\"red\\\":255,\\\"green\\\":255,\\\"blue\\\":0,\\\"guiRgb\\\":16776960,\\\"rgb\\\":65535},\\\"rowSeparators\\\":true,\\\"columnSeparators\\\":true,\\\"rowHeightPixels\\\":13,\\\"labelsHeightPixels\\\":16,\\\"columnConfigs\\\":{\\\"roomType.description\\\":{\\\"ordinal\\\":6,\\\"visible\\\":true,\\\"widthPixels\\\":100,\\\"ehFgColor\\\":null,\\\"ehBgColor\\\":null,\\\"ehAltBgColor\\\":null,\\\"ehLabelFgColor\\\":null,\\\"ehLabelBgColor\\\":null,\\\"ehFont\\\":null,\\\"ehLabelFont\\\":null},\\\"stay.startDate\\\":{\\\"ordinal\\\":3,\\\"visible\\\":true,\\\"widthPixels\\\":51,\\\"ehFgColor\\\":null,\\\"ehBgColor\\\":null,\\\"ehAltBgColor\\\":null,\\\"ehLabelFgColor\\\":null,\\\"ehLabelBgColor\\\":null,\\\"ehFont\\\":null,\\\"ehLabelFont\\\":null},\\\"expr 2(\\\\\\\"integer\\\\\\\", ?, \\\\\\\"\\\\\\\")\\\":{\\\"ordinal\\\":5,\\\"visible\\\":true,\\\"widthPixels\\\":61,\\\"ehFgColor\\\":null,\\\"ehBgColor\\\":null,\\\"ehAltBgColor\\\":null,\\\"ehLabelFgColor\\\":null,\\\"ehLabelBgColor\\\":null,\\\"ehFont\\\":null,\\\"ehLabelFont\\\":null},\\\"stay.price\\\":{\\\"ordinal\\\":7,\\\"visible\\\":true,\\\"widthPixels\\\":48,\\\"ehFgColor\\\":null,\\\"ehBgColor\\\":null,\\\"ehAltBgColor\\\":null,\\\"ehLabelFgColor\\\":null,\\\"ehLabelBgColor\\\":null,\\\"ehFont\\\":null,\\\"ehLabelFont\\\":null},\\\"stay.numGuests\\\":{\\\"ordinal\\\":2,\\\"visible\\\":true,\\\"widthPixels\\\":61,\\\"ehFgColor\\\":null,\\\"ehBgColor\\\":null,\\\"ehAltBgColor\\\":null,\\\"ehLabelFgColor\\\":null,\\\"ehLabelBgColor\\\":null,\\\"ehFont\\\":null,\\\"ehLabelFont\\\":null},\\\"stay.endDate\\\":{\\\"ordinal\\\":4,\\\"visible\\\":true,\\\"widthPixels\\\":51,\\\"ehFgColor\\\":null,\\\"ehBgColor\\\":null,\\\"ehAltBgColor\\\":null,\\\"ehLabelFgColor\\\":null,\\\"ehLabelBgColor\\\":null,\\\"ehFont\\\":null,\\\"ehLabelFont\\\":null},\\\"stay.roomNum\\\":{\\\"ordinal\\\":0,\\\"visible\\\":true,\\\"widthPixels\\\":56,\\\"ehFgColor\\\":null,\\\"ehBgColor\\\":null,\\\"ehAltBgColor\\\":null,\\\"ehLabelFgColor\\\":null,\\\"ehLabelBgColor\\\":null,\\\"ehFont\\\":null,\\\"ehLabelFont\\\":null},\\\"expr 1(\\\\\\\"character\\\\\\\", \\\\\\\"x(40)\\\\\\\", \\\\\\\"\\\\\\\")\\\":{\\\"ordinal\\\":1,\\\"visible\\\":true,\\\"widthPixels\\\":116,\\\"ehFgColor\\\":null,\\\"ehBgColor\\\":null,\\\"ehAltBgColor\\\":null,\\\"ehLabelFgColor\\\":null,\\\"ehLabelBgColor\\\":null,\\\"ehFont\\\":null,\\\"ehLabelFont\\\":null}}}\",\"size\":1858,\"head\":true}"

#143 Updated by Sergey Ivanovskiy almost 4 years ago

Yes, it looks like an issue. How it can be happened?

#144 Updated by Stanislav Lomany almost 4 years ago

Well, I don't like new line symbol within a key and double quoting. But I'm not sure if that's an issue.
So, sorry, again, does your local storage survive server restart?

#145 Updated by Sergey Ivanovskiy almost 4 years ago

Stanislav Lomany wrote:

Well, I don't like new line symbol within a key and double quoting. But I'm not sure if that's an issue.
So, sorry, again, does your local storage survive server restart?

It seems that now the browse configuration is restored. I investigated and found that this function function writeStringBinaryMessage(message, text, offset) didn't change its signature since it had been created. Thus, it is my bug. Sorry. The strange thing is that I tested serialization and deserialization and it should work in my eyes. This diff that you found should fix this deserialization issue

=== modified file 'src/com/goldencode/p2j/ui/client/driver/web/res/p2j.socket.js'
--- src/com/goldencode/p2j/ui/client/driver/web/res/p2j.socket.js    2020-04-08 22:25:30 +0000
+++ src/com/goldencode/p2j/ui/client/driver/web/res/p2j.socket.js    2020-06-19 22:09:55 +0000
@@ -3140,7 +3140,7 @@
             writeInt32BinaryMessage(msg, offset, valueLength);
             offset += 4;

-            writeStringBinaryMessage(msg, offset, value);
+            writeStringBinaryMessage(msg, value, offset);

             send(msg);
             break;
@@ -3202,7 +3202,7 @@
             writeInt32BinaryMessage(msg, offset, keyLength);
             offset += 4;

-            writeStringBinaryMessage(msg, offset, key);
+            writeStringBinaryMessage(msg, key, offset);

             send(msg);
             break;

#146 Updated by Stanislav Lomany almost 4 years ago

#3880-141 Thank you, I know:)

#147 Updated by Stanislav Lomany almost 4 years ago

It seems that now the browse configuration is restored.

For me configuration is still not loaded after server restart because the web local storage gets wiped. Does the same happen on your side?

#148 Updated by Sergey Ivanovskiy almost 4 years ago

At least the cell's background color, height and font were persisted across the server restarts.

#149 Updated by Sergey Ivanovskiy almost 4 years ago

Stanislav Lomany wrote:

It seems that now the browse configuration is restored.

For me configuration is still not loaded after server restart because the web local storage gets wiped. Does the same happen on your side?

No, I didn't observe it, please explain with examples.

#150 Updated by Stanislav Lomany almost 4 years ago

Sorry, I forgot to mention it! Please test it while saving configuration for "This Browse, Current User" (or "All browses, Current User"). Otherwise it is saved on the server side and obviously survives server restart.

#151 Updated by Stanislav Lomany almost 4 years ago

What I see:
  1. Start an app and save a configuration. In local storage browser I see that the configuration is saved under "https://localhost:some_port1"
  2. Restart server an re-login.
  3. In local storage browser I see that the local storage for "https://localhost:some_port2" is empty.

#152 Updated by Sergey Ivanovskiy almost 4 years ago

  • File EnhancedBrowseConfigInlocalStorage.png added

I built FWD with these changes and applied them to Hotel GUI with deploy.prepare. Then cleaned all localStorage and started these tests with several server's restarts. The web client is connected to the same host and port number for each of the server's runs. And tested saving configuration for "This Browse, Current User".

The current key/value pair is
enhancedBrowse/desktop/bogus/guests-frame.w/staysBrowse/expr 1("character", "x(40)", "");expr 2("integer", ?, "");roomType.description;stay.endDate;stay.numGuests;stay.price;stay.roomNum;stay.startDate

"{\"value\":\"{\\\"ehKeyProcName\\\":\\\"guests-frame.w\\\",\\\"ehKeyBrowseName\\\":\\\"staysBrowse\\\",\\\"userName\\\":\\\"bogus\\\",\\\"ehBgColor\\\":{\\\"red\\\":255,\\\"green\\\":255,\\\"blue\\\":128,\\\"guiRgb\\\":16777088,\\\"rgb\\\":8454143},\\\"rowSeparators\\\":true,\\\"columnSeparators\\\":true,\\\"rowHeightPixels\\\":63,\\\"labelsHeightPixels\\\":16,\\\"columnConfigs\\\":{\\\"roomType.description\\\":{\\\"ordinal\\\":6,\\\"visible\\\":true,\\\"widthPixels\\\":100,\\\"ehFgColor\\\":null,\\\"ehBgColor\\\":null,\\\"ehAltBgColor\\\":null,\\\"ehLabelFgColor\\\":null,\\\"ehLabelBgColor\\\":null,\\\"ehFont\\\":null,\\\"ehLabelFont\\\":null},\\\"stay.startDate\\\":{\\\"ordinal\\\":3,\\\"visible\\\":true,\\\"widthPixels\\\":51,\\\"ehFgColor\\\":null,\\\"ehBgColor\\\":null,\\\"ehAltBgColor\\\":null,\\\"ehLabelFgColor\\\":null,\\\"ehLabelBgColor\\\":null,\\\"ehFont\\\":null,\\\"ehLabelFont\\\":null},\\\"expr 2(\\\\\\\"integer\\\\\\\", ?, \\\\\\\"\\\\\\\")\\\":{\\\"ordinal\\\":5,\\\"visible\\\":true,\\\"widthPixels\\\":61,\\\"ehFgColor\\\":null,\\\"ehBgColor\\\":null,\\\"ehAltBgColor\\\":null,\\\"ehLabelFgColor\\\":null,\\\"ehLabelBgColor\\\":null,\\\"ehFont\\\":null,\\\"ehLabelFont\\\":null},\\\"stay.price\\\":{\\\"ordinal\\\":7,\\\"visible\\\":true,\\\"widthPixels\\\":48,\\\"ehFgColor\\\":null,\\\"ehBgColor\\\":null,\\\"ehAltBgColor\\\":null,\\\"ehLabelFgColor\\\":null,\\\"ehLabelBgColor\\\":null,\\\"ehFont\\\":null,\\\"ehLabelFont\\\":null},\\\"stay.numGuests\\\":{\\\"ordinal\\\":2,\\\"visible\\\":true,\\\"widthPixels\\\":61,\\\"ehFgColor\\\":null,\\\"ehBgColor\\\":{\\\"red\\\":255,\\\"green\\\":128,\\\"blue\\\":128,\\\"guiRgb\\\":16744576,\\\"rgb\\\":8421631},\\\"ehAltBgColor\\\":null,\\\"ehLabelFgColor\\\":null,\\\"ehLabelBgColor\\\":null,\\\"ehFont\\\":{\\\"key\\\":\\\"abyssinica sil,10,true,false,false\\\",\\\"fontName\\\":\\\"Abyssinica SIL\\\",\\\"style\\\":\\\"BOLD\\\",\\\"pointSize\\\":10,\\\"legacyWidth\\\":7,\\\"legacyMaxWidth\\\":10,\\\"legacyHeight\\\":14,\\\"fontDefinition\\\":null,\\\"fixedSize\\\":false,\\\"boldFontDefinition\\\":false,\\\"swingAntiAliasing\\\":false,\\\"fontAlias\\\":null},\\\"ehLabelFont\\\":null},\\\"stay.endDate\\\":{\\\"ordinal\\\":4,\\\"visible\\\":true,\\\"widthPixels\\\":51,\\\"ehFgColor\\\":null,\\\"ehBgColor\\\":null,\\\"ehAltBgColor\\\":null,\\\"ehLabelFgColor\\\":null,\\\"ehLabelBgColor\\\":null,\\\"ehFont\\\":null,\\\"ehLabelFont\\\":null},\\\"stay.roomNum\\\":{\\\"ordinal\\\":0,\\\"visible\\\":true,\\\"widthPixels\\\":56,\\\"ehFgColor\\\":null,\\\"ehBgColor\\\":null,\\\"ehAltBgColor\\\":null,\\\"ehLabelFgColor\\\":null,\\\"ehLabelBgColor\\\":null,\\\"ehFont\\\":null,\\\"ehLabelFont\\\":null},\\\"expr 1(\\\\\\\"character\\\\\\\", \\\\\\\"x(40)\\\\\\\", \\\\\\\"\\\\\\\")\\\":{\\\"ordinal\\\":1,\\\"visible\\\":true,\\\"widthPixels\\\":116,\\\"ehFgColor\\\":null,\\\"ehBgColor\\\":null,\\\"ehAltBgColor\\\":null,\\\"ehLabelFgColor\\\":null,\\\"ehLabelBgColor\\\":null,\\\"ehFont\\\":null,\\\"ehLabelFont\\\":null}}}\",\"size\":2186,\"head\":true}"

#154 Updated by Sergey Ivanovskiy almost 4 years ago

  • File deleted (EnhancedBrowseConfigInlocalStorage.png)

#155 Updated by Sergey Ivanovskiy almost 4 years ago

I tested also with saving "All browses, Current User". The result after cold restart was all tables changes background and the particular configuration is applied too for the particular table. Stanislav, we can state that this issue is fixed.

#156 Updated by Stanislav Lomany almost 4 years ago

Sure!
I just need to resolve this ports story for my local configuration.

#157 Updated by Sergey Ivanovskiy almost 4 years ago

I am using these settings for the web client:

          <node class="container" name="portsRange">
           <node class="integer" name="from">
              <node-attribute name="value" value="7449"/>
            </node>
            <node class="integer" name="to">
              <node-attribute name="value" value="7459"/>
            </node>
          </node>

from
        <node class="container" name="webClient">
          <node class="boolean" name="virtualDesktopEnabled">
            <node-attribute name="value" value="TRUE"/>
          </node>
          <node class="boolean" name="embedded">
            <node-attribute name="value" value="FALSE"/>
          </node>
          <node class="boolean" name="enabled">
            <node-attribute name="value" value="TRUE"/>
          </node>
          <node class="string" name="host">
            <node-attribute name="value" value="127.0.0.1"/>
          </node>
          <node class="container" name="portsRange">
            <node class="integer" name="from">
              <node-attribute name="value" value="7449"/>
            </node>
            <node class="integer" name="to">
              <node-attribute name="value" value="7459"/>
            </node>
          </node>
          <node class="integer" name="maxBinaryMessage">
            <node-attribute name="value" value="32894"/>
          </node>
          <node class="integer" name="maxIdleTime">
            <node-attribute name="value" value="90000"/>
          </node>
          <node class="integer" name="watchdogTimeout">
            <node-attribute name="value" value="120000"/>
          </node>
          <node class="container" name="login-form">
            <node class="string" name="background-color">
              <node-attribute name="value" value="0xbbbbbb"/>
            </node>
            <node class="double" name="background-opacity">
              <node-attribute name="value" value="0.8"/>
            </node>
            <node class="string" name="background-image">
              <node-attribute name="value" value="/com/goldencode/hotel/embedded/images/fwd_opacity_60.png"/>
            </node>
            <node class="string" name="image-mime-type">
              <node-attribute name="value" value="image/png"/>
            </node>
            <node class="string" name="background-origin">
              <node-attribute name="value" value="border-box"/>
            </node>
            <node class="string" name="background-position">
              <node-attribute name="value" value="center"/>
            </node>
            <node class="string" name="background-size">
              <node-attribute name="value" value="65%"/>
            </node>
            <node class="string" name="background-repeat">
              <node-attribute name="value" value="no-repeat"/>
            </node>
          </node>
          <node class="container" name="desktop">
            <node class="string" name="background">
              <node-attribute name="value" value="0x000000"/>
            </node>
            <node class="string" name="background-color">
              <node-attribute name="value" value="0xbbbbbb"/>
            </node>
            <node class="string" name="background-image">
              <node-attribute name="value" value="/com/goldencode/hotel/embedded/images/fwd.png"/>
            </node>
            <node class="string" name="image-mime-type">
              <node-attribute name="value" value="image/png"/>
            </node>
            <node class="string" name="background-origin">
              <node-attribute name="value" value="border-box"/>
            </node>
            <node class="string" name="background-position">
              <node-attribute name="value" value="center"/>
            </node>
            <node class="string" name="background-size">
              <node-attribute name="value" value="65%"/>
            </node>
            <node class="string" name="background-repeat">
              <node-attribute name="value" value="no-repeat"/>
            </node>
          </node>
          <node class="boolean" name="override">
            <node-attribute name="value" value="TRUE"/>
          </node>
          <node class="string" name="defaultAccount">
            <node-attribute name="value" value="[OS]"/>
          </node>
        </node>

Port forwarding via Apache Web server is only the solution now for mapping of web clients to their hosts but if you will use the configuration above, then the first web client will use the same open port, the second web client will use the next open port and farther the web spawner follows this algorithm to open new web client.

#158 Updated by Greg Shah almost 4 years ago

Stanislav Lomany wrote:

What I see:
  1. Start an app and save a configuration. In local storage browser I see that the configuration is saved under "https://localhost:some_port1"
  2. Restart server an re-login.
  3. In local storage browser I see that the local storage for "https://localhost:some_port2" is empty.

Yes, this is a real problem in the design of our approach. Unfortunately, I missed this.

I think we need some kind of "port affinity" approach here. The idea: as users connect to the system, we should remember the client port which was assigned previously for that user and reuse it whenever possible. For any scenario where there are at least as many ports in the port range as there are users, then this will result in always using the same port for the given user.

Customers implementing a solution where a port range is shared across more separate users than ports, will break this feature. I can accept this as a limitation right now.

Does anyone see a better alternative?

#159 Updated by Hynek Cihlar almost 4 years ago

Greg Shah wrote:

Does anyone see a better alternative?

Crazy-lazy idea: we could identify the client using the available browser information (agent, resolution, OS, versions, language, time zone, etc.). Little effort, but not reliable.

More-effort idea. Hide the ports behind a reverse proxy. This proxy could be deployed externally or as part of FWD. The embedded proxy would ensure port affinity for the assigned ports.

#160 Updated by Constantin Asofiei almost 4 years ago

Hynek Cihlar wrote:

Greg Shah wrote:

Does anyone see a better alternative?

Crazy-lazy idea: we could identify the client using the available browser information (agent, resolution, OS, versions, language, time zone, etc.). Little effort, but not reliable.

On first access to the FWD login page, why not set a cookie with an UUID to identify the client? And after the client gets logged in, FWD will internally assign the port to that UUID.

#161 Updated by Greg Shah almost 4 years ago

Crazy-lazy idea: we could identify the client using the available browser information (agent, resolution, OS, versions, language, time zone, etc.). Little effort, but not reliable.

Please see #4522-2 for my thoughts on browser fingerprinting. I agree it is unreliable, especially for large enterprises where they have highly standardized browser/platform installations.

On the other hand, I haven't yet come up with a foolproof alternative.

More-effort idea. Hide the ports behind a reverse proxy. This proxy could be deployed externally or as part of FWD. The embedded proxy would ensure port affinity for the assigned ports.

We do in fact already support a reverse-proxy. Please see #2683 and Reverse Proxy.

I hesitate to put this in front of every customer just for this feature. It is added complexity and a slight slowdown for every packet sent (in either direction). That added complexity will cost extra time in setup and also in the diagnosis of problems.

On first access to the FWD login page, why not set a cookie with an UUID to identify the client? And after the client gets logged in, FWD will internally assign the port to that UUID.

This has the down side that two people sharing the same browser will get mapped to the same port and thus also to the same configuration. On the other hand, anything stored in the browser's local storage is probably insecure anyway (can be seen by anyone with a bit of knowledge and access to some dev tools). So this may be OK.

This is similar to the idea that we would be using a "system level" Windows registry key (instead of a user-level registry key).

#162 Updated by Hynek Cihlar almost 4 years ago

Constantin Asofiei wrote:

Hynek Cihlar wrote:

Greg Shah wrote:

Does anyone see a better alternative?

Crazy-lazy idea: we could identify the client using the available browser information (agent, resolution, OS, versions, language, time zone, etc.). Little effort, but not reliable.

On first access to the FWD login page, why not set a cookie with an UUID to identify the client? And after the client gets logged in, FWD will internally assign the port to that UUID.

Wouldn't the cookie be subject of CORS when it would be read on a different port (not mentioning different domain)?

#163 Updated by Hynek Cihlar almost 4 years ago

Hynek Cihlar wrote:

Wouldn't the cookie be subject of CORS when it would be read on a different port (not mentioning different domain)?

I see, it would be only needed on the login page. So this would not be a cross-origin access.

#164 Updated by Greg Shah almost 4 years ago

Correct.

A variant of this idea: if the initial URL had a user-account component (https://<host>:<port>/gui/<os_userid>), then we could map this same UUID to the user-level with little difference in the FWD code (other than needing to handle this variation in the URL).

#165 Updated by Hynek Cihlar almost 4 years ago

Greg Shah wrote:

Correct.

A variant of this idea: if the initial URL had a user-account component (https://<host>:<port>/gui/<os_userid>), then we could map this same UUID to the user-level with little difference in the FWD code (other than needing to handle this variation in the URL).

We already have the name. The user sends it during log-in.

#166 Updated by Greg Shah almost 4 years ago

Yes, but by doing it this way the local storage would be specific to the user instead of the machine. It isn't secure but it at least could offer a solution to customers that want the same browser to be used for multiple users.

It also would be useful to allow bookmarks that got a login page that does not require specifying the user name.

#167 Updated by Hynek Cihlar almost 4 years ago

Greg Shah wrote:

Yes, but by doing it this way the local storage would be specific to the user instead of the machine. It isn't secure but it at least could offer a solution to customers that want the same browser to be used for multiple users.

The user is not known until he logs in. So the initial login page can not contain the user name.

#168 Updated by Greg Shah almost 4 years ago

The idea is that the user would have the URL bookmarked which would map to the right user name. Then the login form would only prompt for the password. The servlet code would know how to handle the URL form properly.

#169 Updated by Greg Shah almost 4 years ago

The generic (no user version) of the URL should continue to work as well.

#170 Updated by Greg Shah almost 4 years ago

Task branch 4231b has been merged to trunk as revision 11347.

#171 Updated by Stanislav Lomany almost 4 years ago

Greg, I have a question about the "tray" which should appear if there are no scrollbars and we want to find a place for the gear icon. When a browse is defined as X DOWN, should we extend the browse size to display X rows or we should go for compatibility and display, say, X-1 rows, but the browse occupies the same space in the frame? Also, should we have for this case (no scrollbars) a separate "tray" widget or it should be implemented within ScrollPaneGuiImpl?

#172 Updated by Greg Shah almost 4 years ago

We should not take up more space than the 4GL compatible version. Instead we "sacrifice" a row.

X-1 rows, but the browse occupies the same space in the frame

This one.

Also, should we have for this case (no scrollbars) a separate "tray" widget or it should be implemented within ScrollPaneGuiImpl?

I don't know the answer here, but I wouldn't want a generic helper class to have a browse-specific feature.

#173 Updated by Stanislav Lomany almost 4 years ago

I don't know the answer here, but I wouldn't want a generic helper class to have a browse-specific feature.

If there's a scrollbar, functionality goes into ScrollPaneGuiImpl. Or should I subclass it?

#174 Updated by Greg Shah almost 4 years ago

You can subclass it if that is the best way. But it seems like the gear icon will always be there in this mode, so why put it in a separate class that only is related to scrolling? This is a function of the GUI browse itself in enhanced mode. Can't the scrollbars be sized and positioned so that they don't overlap the space for the gear icon?

#175 Updated by Stanislav Lomany almost 4 years ago

Can't the scrollbars be sized and positioned so that they don't overlap the space for the gear icon?

Sorry, don't quite get the question.
Overall, I wasn't going to subclass it.

#176 Updated by Greg Shah almost 4 years ago

I don't think it makes sense to have an enhanced browse menu button inside a scrolling helper class.

#177 Updated by Greg Shah almost 4 years ago

I assume that ScrollPaneGuiImpl needs a change that gives more control over its positioning, sizing or over the drawing of that dead spot, it is OK. But the drawing and event processing for that dead spot needs to be in browse.

#178 Updated by Greg Shah over 3 years ago

#181 Updated by Greg Shah about 3 years ago

In #5145-11, Vladimir makes a very good point that using JasperReports for BROWSE spreadsheet export has a big disadvantage: it forces a paging implementation which does not match with the expectations of a spreadsheet.

In fact, the current implementation of our enhanced browse export does have this very issue, where there are pages implemented instead of a simpler, pure cell approach. Vladimir points out that Apache POI can be used for this and that the code is much simpler/smaller. He wrote an export in just 1 day using this for a customer-specific widget in #3820.

We even have an open item in this task that is related. Item 2.3 of #3880-4 is described as "Deal with multi-page Jasper layout (this relates to the case where export of a browse is too large to reasonably fit on a single page, even in landscape mode, so the output must be split onto multiple pages).".

I think we should consider replacing the current JasperReports approach with the Apache POI approach, if we can indeed implement all the needed features using it.

Vladimir: Can we implement background colors, fonts (including colors) and other layout/visual features using Apache POI?

Stanislav: Do you have any opinions on this?

#182 Updated by Stanislav Lomany about 3 years ago

Stanislav: Do you have any opinions on this?

I don't like dealing with JasperReport templates because the code looks unpleasant, so I'm for the code which is "much simpler/smaller".

#183 Updated by Greg Shah about 3 years ago

I've changed item 2.3 from "Deal with multi-page Jasper layout (this relates to the case where export of a browse is too large to reasonably fit on a single page, even in landscape mode, so the output must be split onto multiple pages)." to "Replace JasperReports export approach with one based on Apache POI which will eliminate the page-related approach in favor of a cell and row based output that customers would expect to see.".

We will move this way unless Vladimir suggests that the visual formatting possibilities cannot be handled in POI.

#185 Updated by Stanislav Lomany over 2 years ago

Add a DISABLE-USER-CFG-EDIT browse option to disable the ability of a user to change any of the otherwise customizable configuration on a per-browse basis. This means that all of the colors, fonts, sizing, column hiding, reorder columns... would be treated as read-only with no ability for a user to modify the values. The vallues would be set from the directory as normal OR would be overridden by any 4GL syntax enhancements used.

Greg, to be sure, are we taking about the following kind of logic?
  1. All user-level ("This Browse/Current User" and "All Browses/Current User") setting are not applied to the given browse for non-admin and admin users.
  2. Abilities to modify "This Browse/Current User" and "All Browses/Current User" settings are disabled for the given browse ("All Browses/Current User" technically can work, but the settings are not applied to the current browse, so it doesn't make much sense to leave this option).
  3. "This Browse/All User" and "All Browses/All Users" work as usual.

#186 Updated by Stanislav Lomany over 2 years ago

3821c rev 12741 adds conversion support for DISABLE-ROW-STRIPING, DISABLE-USER-CFG-EDIT, UNIQUE-KEY and DB-FIELD-MAPPING and runtime support for UNIQUE-KEY and DISABLE-ROW-STRIPING.

#187 Updated by Greg Shah over 2 years ago

  1. All user-level ("This Browse/Current User" and "All Browses/Current User") setting are not applied to the given browse for non-admin and admin users.

No. All enhancements that are in the directory or set via 4GL syntax should be displayed. The DISABLE-USER-CFG-EDIT only affects the editing of settings. It has no impact on displaying/using the settings that are already there.

  1. Abilities to modify "This Browse/Current User" and "All Browses/Current User" settings are disabled for the given browse ("All Browses/Current User" technically can work, but the settings are not applied to the current browse, so it doesn't make much sense to leave this option).

See above.

  1. "This Browse/All User" and "All Browses/All Users" work as usual.

If you mean that admin users should still be able to edit, I think the answer is yes.

#188 Updated by Greg Shah over 2 years ago

Code Review Task Brnach 3821c Revision 12741

I have no objections.

Don't we need some code in the drawing layer to disable the row striping?

#189 Updated by Stanislav Lomany over 2 years ago

No. All enhancements that are in the directory or set via 4GL syntax should be displayed. The DISABLE-USER-CFG-EDIT only affects the editing of settings. It has no impact on displaying/using the settings that are already there.

"All user" settings are stored in the directory. "Current user" setting are saved in local storage. If configuration for the current user exists in local storage, should I apply it?

#190 Updated by Stanislav Lomany over 2 years ago

Don't we need some code in the drawing layer to disable the row striping?

I thought about it. I chose the current approach because the sequence "Save striping -> Disable striping in code -> Save browse settings -> Enable striping in code" leads to no striping rather than bringing striping back.
In other terms there's no need to add duplicate logic.

#191 Updated by Greg Shah over 2 years ago

"Current user" setting are saved in local storage. If configuration for the current user exists in local storage, should I apply it?

No, it can be ignored. This option tells us that the application code does not ever want a user to edit the enhanced configuration. So even if it is there, it should not be used.

#192 Updated by Stanislav Lomany over 2 years ago

So, we disable reading/editing of "This Browse/Current User" and "All Browses/Current User". And leave "This Browse/All User" and "All Browses/All Users" as is, right?

#193 Updated by Greg Shah over 2 years ago

we disable reading/editing of "This Browse/Current User"

Yes.

and "All Browses/Current User".

Actually, I'm not sure about this. The DISABLE-CFG-EDIT is about this browse instance only. What is the reason to disable global browse cfg editing? Is it just to be consistent in disabling editing? If so, then yes.

And leave "This Browse/All User" and "All Browses/All Users" as is,

These are only enabled for admin users, right? So they need to stay enabled for admins and be disabled for regular users, which is how they already work, right? If so, then yes.

#194 Updated by Stanislav Lomany over 2 years ago

and "All Browses/Current User". What is the reason to disable global browse cfg editing? Is it just to be consistent in disabling editing? If so, then yes.

"All Browses/Current User" as a user-level configuration is not applied to the given browse. There's no point to enable editing of it for this browse if it cannot be applied.

And leave "This Browse/All User" and "All Browses/All Users" as is,

These are only enabled for admin users, right?

These are edited by admin users and applied to all users.

Also available in: Atom PDF