View Issue Details

IDProjectCategoryView StatusLast Update
0037927LazarusLCLpublic2020-10-25 00:09
ReporterPedro Gimeno Assigned Towp  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version2.1 (SVN) 
Summary0037927: TIpHtmlPanel does not allow creating controls on the fly
DescriptionThis is probably more of a feature request than a bug, not sure. Currently, dynamically creating a node that is associated to a control (e.g. a TIpHtmlNodeBUTTON node), does not create the associated control.
Steps To ReproducePlace this in the FormCreate of a form of type TForm1 that contains a TIpHtmlPanel called IpHtmlPanel1:

procedure TForm1.FormCreate(Sender: TObject);
var
  Body: TIpHtmlNode;
  Node: TIpHtmlNode;
begin
  IpHtmlPanel1.SetHtmlFromStr(0000060'body>'
    + 'this works: '0000060'button type="button" value="hi">'0000060'/button> but'0000060'br>'
    + 0000060'/body>');
  Body := IpHtmlPanel1.MasterFrame.Html.HtmlNode.ChildNode[0];
  TIpHtmlNodeText.Create(Body).ANSIText := 'this doesn''t: ';
  TIpHtmlNodeBUTTON.Create(Body).value := 'hi';
  TIpHtmlNodeText.Create(Body).ANSIText := ' see?';
end;

Additional InformationThe reason is that all controls are created in a single place, namely in the InternalCreateFrames function, so the code is not really prepared to handle dynamic addition of controls.
TagsipHTMLPanel
Fixed in Revisionr64045, 64051, 64059
LazTarget2.2
Widgetset
Attached Files

Activities

Pedro Gimeno

2020-10-15 12:41

reporter   ~0126324

Sorry, I can't edit to fix the formatting. Here's the example again:

procedure TForm1.FormCreate(Sender: TObject);
var
  Body: TIpHtmlNode;
  Node: TIpHtmlNode;
begin
  IpHtmlPanel1.SetHtmlFromStr(#$3C'body>'
    + 'this works: '#$3C'button type="button" value="hi">'#$3C'/button> but'#$3C'br>'
    + #$3C'/body>');
  Body := IpHtmlPanel1.MasterFrame.Html.HtmlNode.ChildNode[0];
  TIpHtmlNodeText.Create(Body).ANSIText := 'this doesn''t: ';
  TIpHtmlNodeBUTTON.Create(Body).value := 'hi';
  TIpHtmlNodeText.Create(Body).ANSIText := ' see?';
end;

Zaher Dirkey

2020-10-18 11:22

reporter   ~0126386

This bug need discussing, I fixed it with my patch but i am not stratified about it, ipHTMLPanel is complex to understand it.

each node when created on the fly, (after loading) If it a control (Button/Input) it need a parent Panel (hyperpanel here), but how can I reach this panel?
I added it as property to ipHTML (I dont like it, we need more complex idea to be suite with ipHTML complexity :P )
iphtml.pas05.patch (1,516 bytes)   
Index: components/turbopower_ipro/iphtml.pas
===================================================================
--- components/turbopower_ipro/iphtml.pas	(revision 64043)
+++ components/turbopower_ipro/iphtml.pas	(working copy)
@@ -1994,6 +1994,8 @@
   TControlEvent2 = procedure(Sender: TIpHtml; Node: TIpHtmlNodeControl; var cancel: boolean)
     of object;
 
+  TIpHtmlInternalPanel = class;
+
   TIpHtml = class
   private
     FHotNode : TIpHtmlNode;
@@ -2091,6 +2093,7 @@
     FStartSel, FEndSel : TPoint;
     ElementPool : TIpHtmlPoolManager;
     AnchorList : {$ifdef IP_LAZARUS}TFPList{$else}TList{$endif};
+    HyperPanel: TIpHtmlInternalPanel;
     FControlList : {$ifdef IP_LAZARUS}TFPList{$else}TList{$endif};
     FCurURL : string;
     DoneLoading : Boolean;
@@ -2375,8 +2378,6 @@
   end;
   {$ENDIF}
 
-  TIpHtmlInternalPanel = class;
-
   TIpHtmlScrollBar = class
   private
     FKind: TScrollBarKind;
@@ -13510,6 +13511,10 @@
   inherited Create(ParentNode);
   Owner.FControlList.Add(Self);
   Align := hiaBottom;
+  if Owner.DoneLoading then
+  begin
+    CreateControl(Owner.HyperPanel);
+  end;
 end;
 
 destructor TIpHtmlNodeControl.Destroy;
@@ -15051,6 +15056,7 @@
     HyperPanel.OnHotClick := FViewer.HotClick;
     HyperPanel.OnClick := FViewer.ClientClick;
     HyperPanel.TabStop := FViewer.WantTabs;
+    FHtml.HyperPanel := HyperPanel;
     FHtml.OnScroll := HyperPanel.ScrollRequest;
     FHtml.OnControlClick := ControlClick;
     FHtml.OnControlClick2 := ControlClick2;
iphtml.pas05.patch (1,516 bytes)   

Pedro Gimeno

2020-10-19 17:45

reporter   ~0126417

The patch works for my usage, thanks a lot. The button does not receive the intended caption because changing it at run time via the HTML node's properties is not supported, but the underlying TButton can be accessed through the Control property, and then its caption can be adjusted.

Zaher Dirkey

2020-10-20 14:52

reporter   ~0126423

Than mean we need to add property Control to each html element/node to link it then change it when a property of that element changed, i am afraid that can point to destroyed objects in somehow , maybe controls regenerated again, I need an idea.

Zaher Dirkey

2020-10-20 15:19

reporter   ~0126424

The idea behind that we can make forms with controls, instead of using lfm, we can use HTML style.
to solve it, we need to be brave to make more bugs and testing it, I can't do that in official branch.
There is 3 important problems I need to slove it,
1 - Unicode
2 - Bidi (Direction for RTL languages
3 - Add partial html node using html not creating elements. like Body.Add('

New line

')
4 - oh there is 4, export as html, (I cant find it)

Maybe we need to move this topic to the forum?

wp

2020-10-20 17:34

developer   ~0126427

Last edited: 2020-10-20 17:35

View 3 revisions

Debugging your application I found that CreateControl is called inside the loading squence: IpHtmlPanel.SetHtmlFromStr calls SethtmlFromStream - this creates an IpHtml which loads the stream. This, in turn, calls TIpHtmlCustomPanel.SetHtml and FMasterFrame.SetHtml. The latter calls TIpHtmlFrame.InternalCreateFrame where the controls are added (CreateControl)

So, when you simply create controls by calling their constructor a lot of the work is missing.

I'd recommend to create an IpHtml, load the html text to it and add the on-the-fly controls here. Then put this into the SetHtml method of the IpHtmlPanel.

procedure TForm1.FormCreate(Sender: TObject);
var
  stream: TStringStream;

  Body: TIpHtmlNode;
  html: TIpHtml;
begin
  stream := TStringStream.Create('<body>'
    + 'this works: <button type="button" value="hi"></button> but '
    + '</body>');
  html := TIpHtml.Create;
  html.LoadfromStream(stream);
  stream.Free;

  body := html.HtmlNode.ChildNode[0];
  TIpHtmlNodeText.Create(body).ANSIText := 'this doesn''t: ';
  TIpHtmlNodeBUTTON.Create(body).Value := 'hi';
  TIpHtmlNodeText.Create(Body).ANSIText := ' see?';

  IpHtmlPanel1.SetHtml(html);
end;

Zaher Dirkey

2020-10-20 21:01

reporter   ~0126430

@wp creating it on the fly it is like ajax, adding new element witout reloading it again, in your example it will make flicker, for chat projects, log etc.
I have a real project using it, but i will check your comment again

@Pedro, @wp

I fixed assigning Value, and other properties of node/element of button, in my next patch, please check it
iphtml.pas06.patch (7,634 bytes)   
Index: components/turbopower_ipro/iphtml.pas
===================================================================
--- components/turbopower_ipro/iphtml.pas	(revision 64043)
+++ components/turbopower_ipro/iphtml.pas	(working copy)
@@ -1810,6 +1810,8 @@
 
   TIpHtmlButtonType = (hbtSubmit, hbtReset, hbtButton);
 
+  { TIpHtmlNodeBUTTON }
+
   TIpHtmlNodeBUTTON = class(TIpHtmlNodeControl)
   private
     FDisabled: Boolean;
@@ -1817,9 +1819,11 @@
     FValue: string;
     FName: string;
     FInputType: TIpHtmlButtonType;
+    procedure SetDisabled(AValue: Boolean);
+    procedure SetInputType(AValue: TIpHtmlButtonType);
+    procedure SetValue(AValue: string);
   protected
-    procedure SubmitClick(Sender: TObject);
-    procedure ResetClick(Sender: TObject);
+    procedure RecalcSize;
     procedure ButtonClick(Sender: TObject);
     function Successful: Boolean; override;
     procedure AddValues(NameList, ValueList : TStringList); override;
@@ -1831,11 +1835,11 @@
   {$IFDEF HTML_RTTI}
   published
   {$ENDIF}
-    property ButtonType : TIpHtmlButtonType read FInputType write FInputType;
-    property Disabled : Boolean read FDisabled write FDisabled;
-    property Name : string read FName write FName;
+  property Name : string read FName write FName;
+    property ButtonType : TIpHtmlButtonType read FInputType write SetInputType;
+    property Disabled : Boolean read FDisabled write SetDisabled;
     property TabIndex : Integer read FTabIndex write FTabIndex;
-    property Value : string read FValue write FValue;
+    property Value : string read FValue write SetValue;
   end;
 
   TIpHtmlNodeSELECT = class(TIpHtmlNodeControl)
@@ -1994,6 +1998,8 @@
   TControlEvent2 = procedure(Sender: TIpHtml; Node: TIpHtmlNodeControl; var cancel: boolean)
     of object;
 
+  TIpHtmlInternalPanel = class;
+
   TIpHtml = class
   private
     FHotNode : TIpHtmlNode;
@@ -2054,7 +2060,6 @@
     FPageViewBottom : Integer; {the lower end of the page, may be different from PageViewRect.Bottom }
     FPageViewTop: Integer; { the upper end of the page }
     DefaultProps : TIpHtmlProps;
-    Body : TIpHtmlNodeBODY;
     FTitleNode : TIpHtmlNodeTITLE;
    {$IFDEF IP_LAZARUS}
       FDataProvider: TIpAbstractHtmlDataProvider;
@@ -2091,6 +2096,7 @@
     FStartSel, FEndSel : TPoint;
     ElementPool : TIpHtmlPoolManager;
     AnchorList : {$ifdef IP_LAZARUS}TFPList{$else}TList{$endif};
+    HyperPanel: TIpHtmlInternalPanel;
     FControlList : {$ifdef IP_LAZARUS}TFPList{$else}TList{$endif};
     FCurURL : string;
     DoneLoading : Boolean;
@@ -2313,6 +2319,7 @@
     function getControlCount:integer;
     function getControl(i:integer):TIpHtmlNode;
   public
+    Body : TIpHtmlNodeBODY;
     constructor Create;
     destructor Destroy; override;
     function PagePtToScreen(const Pt: TPoint): TPoint;
@@ -2375,8 +2382,6 @@
   end;
   {$ENDIF}
 
-  TIpHtmlInternalPanel = class;
-
   TIpHtmlScrollBar = class
   private
     FKind: TScrollBarKind;
@@ -12018,6 +12023,7 @@
   end;
 
 begin
+  inherited;
   Owner.ControlCreate(Self);
   aCanvas := TFriendPanel(Parent).Canvas;
   iCurFontSize := aCanvas.Font.Size;
@@ -12435,6 +12441,7 @@
   i, j, iCurFontSize: integer;
   OptGroup: TIpHtmlNodeOPTGROUP;
 begin
+  inherited;
   Owner.ControlCreate(Self);
   aCanvas := TFriendPanel(Parent).Canvas;
   iCurFontSize := aCanvas.Font.Size;
@@ -12599,6 +12606,7 @@
   iCurFontSize: integer;
   aCanvas : TCanvas;
 begin
+  inherited;
   Owner.ControlCreate(Self);
   aCanvas := TFriendPanel(Parent).Canvas;
   iCurFontSize := aCanvas.Font.Size;
@@ -13222,23 +13230,18 @@
 begin
   inherited Create(ParentNode);
   FElementName := 'button';
-  Owner.FControlList.Add(Self);
 end;
 
 destructor TIpHtmlNodeBUTTON.Destroy;
 begin
-  Owner.FControlList.Remove(Self);
   inherited;
 end;
 
 procedure TIpHtmlNodeBUTTON.CreateControl(Parent: TWinControl);
-var
-   iCurFontSize: integer;
-   aCanvas : TCanvas;
 begin
+  inherited;
   Owner.ControlCreate(Self);
-  aCanvas := TFriendPanel(Parent).Canvas;
-  iCurFontSize := aCanvas.Font.Size;
+
   FControl := TButton.Create(Parent);
   FControl.Visible := False;
   FControl.Parent := Parent;
@@ -13247,28 +13250,9 @@
   with TButton(FControl) do begin
     Enabled := not Self.Disabled;
     Caption := Value;
-    case ButtonType of
-    hbtSubmit :
-      begin
-        OnClick := SubmitClick;
-        if Caption = '' then
-          Caption := SHtmlDefSubmitCaption;
-      end;
-    hbtReset :
-      begin
-        OnClick := ResetClick;
-        if Caption = '' then
-          Caption := SHtmlDefResetCaption;
-      end;
-    hbtButton :
-      begin
-        OnClick := ButtonClick;
-      end;
-    end;
-    Width := TFriendPanel(Parent).Canvas.TextWidth(Caption) + 40;
-    Height := TFriendPanel(Parent).Canvas.TextHeight(Caption) + 10;
+    OnClick := ButtonClick;
   end;
-  aCanvas.Font.Size := iCurFontSize;
+  RecalcSize;
 end;
 
 procedure TIpHtmlNodeBUTTON.Reset;
@@ -13275,19 +13259,73 @@
 begin
 end;
 
-procedure TIpHtmlNodeBUTTON.ResetClick(Sender: TObject);
+procedure TIpHtmlNodeBUTTON.SetValue(AValue: string);
 begin
-  ResetRequest;
+  if FValue =AValue then Exit;
+  FValue :=AValue;
+  if Owner.DoneLoading and (FControl <> nil) then
+  begin
+    (FControl as TButton).Caption := AValue;
+    RecalcSize;
+  end;
 end;
 
-procedure TIpHtmlNodeBUTTON.SubmitClick(Sender: TObject);
+procedure TIpHtmlNodeBUTTON.RecalcSize;
+var
+   OldFontSize: integer;
+   aCanvas : TCanvas;
 begin
-  SubmitRequest;
+  with Control as TButton do
+  begin
+    aCanvas := TFriendPanel(Parent).Canvas;
+    OldFontSize := aCanvas.Font.Size;
+    Width := TFriendPanel(Parent).Canvas.TextWidth(Caption) + 40;
+    Height := TFriendPanel(Parent).Canvas.TextHeight(Caption) + 10;
+    aCanvas.Font.Size := OldFontSize;
+  end;
 end;
 
+procedure TIpHtmlNodeBUTTON.SetDisabled(AValue: Boolean);
+begin
+  if FDisabled =AValue then Exit;
+  FDisabled :=AValue;
+  if Owner.DoneLoading and (FControl <> nil) then
+    (FControl as TButton).Enabled := not AValue;
+end;
+
+procedure TIpHtmlNodeBUTTON.SetInputType(AValue: TIpHtmlButtonType);
+begin
+  //if FInputType =AValue then Exit; //nop we need it to set default caption
+  FInputType := AValue;
+  if Owner.DoneLoading and (FControl <> nil) then
+      if Self.Value = '' then //Value = Caption here
+        case FInputType of
+        hbtSubmit :
+           Self.Value := SHtmlDefSubmitCaption;
+        hbtReset :
+           Self.Value := SHtmlDefResetCaption;
+        hbtButton :
+          begin
+          end;
+        end;
+end;
+
 procedure TIpHtmlNodeBUTTON.ButtonClick(Sender: TObject);
 begin
-  Owner.ControlClick(Self);
+  case ButtonType of
+  hbtSubmit :
+    begin
+      SubmitRequest;
+    end;
+  hbtReset :
+    begin
+      ResetRequest;
+    end;
+  hbtButton :
+    begin
+      Owner.ControlClick(Self);
+    end;
+  end;
 end;
 
 function TIpHtmlNodeBUTTON.Successful: Boolean;
@@ -13510,6 +13548,10 @@
   inherited Create(ParentNode);
   Owner.FControlList.Add(Self);
   Align := hiaBottom;
+  if Owner.DoneLoading then
+  begin
+    CreateControl(Owner.HyperPanel);
+  end;
 end;
 
 destructor TIpHtmlNodeControl.Destroy;
@@ -15051,6 +15093,7 @@
     HyperPanel.OnHotClick := FViewer.HotClick;
     HyperPanel.OnClick := FViewer.ClientClick;
     HyperPanel.TabStop := FViewer.WantTabs;
+    FHtml.HyperPanel := HyperPanel; //TODO i dont like it, it used when creating control element on the fly after html loaded
     FHtml.OnScroll := HyperPanel.ScrollRequest;
     FHtml.OnControlClick := ControlClick;
     FHtml.OnControlClick2 := ControlClick2;
iphtml.pas06.patch (7,634 bytes)   

wp

2020-10-20 22:55

developer   ~0126434

I am not sure but I think that your patches can introduce issues later.

TIpHtmlCustomPanel (and TIpHtmlPanel) has a field MasterFrame, type TIpHtmlFrame

A TIpHtmlFrame (a simple class) contains (among others) these fields
- Html, type TIpHtml
- HyperPanel, type TIpHtmlInternalPanel
- Frames: array of TIpHtmlFrame

The HyperPanel, type TIpHtmlInternalPanel inheriting from TCustomControl, contains a field Hyper which is type TIpHtml.

The class TIpHtml has no backreferences to any of these classes. This means to me that TIpHtml and all the nodes are standalone and in principle can be separated off of the IpHtml unit (which sooner or later must be done - this huge unit simply is not manageable).

But now your patch adds the HyperPanel, class TIpHtmlInternalPanel, which links back to all the visual components.This will make it more difficult to separate the visual from the html code. Since HyperPanel would be needed by the declaration of TIpHtml and TIpHtml would be needed by the declaration of TIpHtmlInternalPanel (the class of HyperPanel) we have a perfect cricular reference when both are in separate units. This can only be resolved by declaring HyperPanel in TIpHtml as its ancestor TCustomControl and using ugly typecasts in the implementation part of the unit.

The current architecture also makes it possible that the same IpHtml instance can be used in several IpHtmlFrames. I don't know if this situation can ever occur, but adding the HyperPanel field makes this definitely impossible.

So, I think that your patches will limit extendibility and refactoring of the IpHtmlPanel.

Pedro Gimeno

2020-10-20 22:58

reporter   ~0126435

@wp

> So, when you simply create controls by calling their constructor a lot of the work is missing.

Fortunately, the bulk of that work is done by CreateControl, and the only part missing is setting the parent to the HyperPanel. Check Zaher's first patch; I don't think there's any other action missing. Well, besides setting the properties, which can't be edited for existing controls anyway (that's the subject of Zaher's second patch).

Reloading the HTML from the start every time some text is added, is too costly. I need it for a chat log where some controls need to be created sometimes. In a chat log, adding a new line shouldn't mean that everything has to be re-interpreted and re-rendered from the beginning, especially when the backlog gets long; I need an incremental solution. My fallback solution is to use hyperlinks instead of buttons, but I'd rather have real buttons if possible.

Pedro Gimeno

2020-10-21 11:28

reporter   ~0126442

The circularity can easily be removed by making the HyperPanel in the patch be of type TWinControl instead of TIpHtmlCustomPanel.

wp

2020-10-21 20:09

developer   ~0126445

Last edited: 2020-10-21 20:11

View 4 revisions

In r64045 I committed a possible solution based on Zaher Dirkey's patch. It does not add the HyperPanel as an element to the TIpHtml class, but introduces a new method of TIpHtmlCustomPanel, AddNodeControl, which must be called with the created button node as a parameter. Internally it simply calls CreateControl. I think this avoids all the possible issues that I mentioned above:

  ButtonNode := TIpHtmlNodeBUTTON.Create(Body);
  ButtonNode.Value := 'hello world';
  ButtonNode.ButtonType := hbtButton;
  IpHtmlPanel1.AddNodeControl(ButtonNode); 

Please test carefully. I am attaching the test program that I used.

Although a bit less comfortable than Zaher's solution, this is similar to the LCL where you first create a control and then add it to container by setting its Parent.

There are still issues:
- When the button caption is changed the layout should be updated; the current version is overwrites the following text node in the demo program.

Zaher Dirkey

2020-10-21 21:23

reporter   ~0126447

"The circularity can easily be removed by making the HyperPanel in the patch be of type TWinControl instead of TIpHtmlCustomPanel."
Yes, and It is more logical to rename HyperPanel to Control: TWinControl, as like as TIpHtmlNodeBUTTON/TIpHtmlNodeControl have Control: TWinControl, and maybe make TipHTML node class derived from same class of TIpHtmlNodeControl.

That make both nodes will have Control property, Parent control (TipHTML) and the childs nodes (TIpHtmlNodeBUTTON like).

@wp, I tried this idea (AddNodeControl(ButtonNode); before doing my patch, I dont like it, it is mean more work for end user (programmer who use ipHTML), and no one will guess he should add this function.

Zaher Dirkey

2020-10-21 21:35

reporter   ~0126448

Solution should be more easy for programmer like this :P
TIpHtmlNodeBUTTON.Create(Body, 'Submit', hbtButton);

wp

2020-10-21 22:41

developer   ~0126449

Hey come on, What's the problem of typing a few lines? Do you want to return to the times of calling procedures with myriads of parameters where nobody knows what they mean?

Yes, nobody will guess to call this function at the moment. But you two who seem to be the only users of this feature know now. Before reading this report I was not even aware that the IpHtmlPanel could be used for something like what is discussed here. It's just a matter of documentation. What is missing is a good wiki tutorial. When it mentions creation of html controls on the fly every user will know that he has to call this method, just like an LCL user knows that he must set the parent of a run-time created control.

I did not write the TurboPower IPro lib, and I must say that I barely understand what is happening in detail. Therefore I am against changes in the depths of the library because I cannot guarantee that it will not break anything else. The AddNodeControl method is harmless. It is not called anywhere in the normal use case, it thus cannot break anything else. Adding the HyperPanel to TIpHtml, however, destroys the logical interdependence of the classes. It may not be harmful now but it may be in the future.

But let's return to work, your changes do have consequences. We're only talking of the buttons here. But there are other nodes that inherit from TIpHtmlNodeControl and should suffer from the same problem in the first place. Did somebody test them? (I did not because I don't have the time for it). They SHOULD work when AddNodeControl is called but bugs are treacherous. I will not be happy resolving this report with the button working but crashing for the INPUT node. What with the autosizing of the button when the caption is changed so that following html elements can be covered? I think changing the button caption should trigger a re-layout of the following html text - which of course will cause flicker. Or is it an unnecessary requirement to be able to change the caption of a button?

Zaher Dirkey

2020-10-21 22:57

reporter   ~0126450

"Therefore I am against changes in the depths of the library because I cannot guarantee that it will not break anything else. The AddNodeControl method is harmless."
I agree

"But there are other nodes that inherit from TIpHtmlNodeControl and should suffer from the same problem in the first place."
I wanted to change it all, but not in the first patch, that maybe rejected

"What with the autosizing of the button when the caption is changed so that following html elements can be covered?"
Already done in last patch.

and I agree it is need more testing in a real project.

Zaher Dirkey

2020-10-21 23:11

reporter   ~0126451

This is my last patch before I revert my changes to keep code as official

I Just renamed HyperPanel to Control: TWinControl
iphtml.pas07.patch (8,016 bytes)   
Index: components/turbopower_ipro/iphtml.pas
===================================================================
--- components/turbopower_ipro/iphtml.pas	(revision 64043)
+++ components/turbopower_ipro/iphtml.pas	(working copy)
@@ -1810,6 +1810,8 @@
 
   TIpHtmlButtonType = (hbtSubmit, hbtReset, hbtButton);
 
+  { TIpHtmlNodeBUTTON }
+
   TIpHtmlNodeBUTTON = class(TIpHtmlNodeControl)
   private
     FDisabled: Boolean;
@@ -1817,9 +1819,11 @@
     FValue: string;
     FName: string;
     FInputType: TIpHtmlButtonType;
+    procedure SetDisabled(AValue: Boolean);
+    procedure SetInputType(AValue: TIpHtmlButtonType);
+    procedure SetValue(AValue: string);
   protected
-    procedure SubmitClick(Sender: TObject);
-    procedure ResetClick(Sender: TObject);
+    procedure RecalcSize;
     procedure ButtonClick(Sender: TObject);
     function Successful: Boolean; override;
     procedure AddValues(NameList, ValueList : TStringList); override;
@@ -1831,11 +1835,11 @@
   {$IFDEF HTML_RTTI}
   published
   {$ENDIF}
-    property ButtonType : TIpHtmlButtonType read FInputType write FInputType;
-    property Disabled : Boolean read FDisabled write FDisabled;
-    property Name : string read FName write FName;
+  property Name : string read FName write FName;
+    property ButtonType : TIpHtmlButtonType read FInputType write SetInputType;
+    property Disabled : Boolean read FDisabled write SetDisabled;
     property TabIndex : Integer read FTabIndex write FTabIndex;
-    property Value : string read FValue write FValue;
+    property Value : string read FValue write SetValue;
   end;
 
   TIpHtmlNodeSELECT = class(TIpHtmlNodeControl)
@@ -1994,7 +1998,9 @@
   TControlEvent2 = procedure(Sender: TIpHtml; Node: TIpHtmlNodeControl; var cancel: boolean)
     of object;
 
-  TIpHtml = class
+  TIpHtmlInternalPanel = class;
+
+  TIpHtml = class(TObject)
   private
     FHotNode : TIpHtmlNode;
     FCurElement : PIpHtmlElement;
@@ -2054,7 +2060,6 @@
     FPageViewBottom : Integer; {the lower end of the page, may be different from PageViewRect.Bottom }
     FPageViewTop: Integer; { the upper end of the page }
     DefaultProps : TIpHtmlProps;
-    Body : TIpHtmlNodeBODY;
     FTitleNode : TIpHtmlNodeTITLE;
    {$IFDEF IP_LAZARUS}
       FDataProvider: TIpAbstractHtmlDataProvider;
@@ -2091,6 +2096,7 @@
     FStartSel, FEndSel : TPoint;
     ElementPool : TIpHtmlPoolManager;
     AnchorList : {$ifdef IP_LAZARUS}TFPList{$else}TList{$endif};
+    FControl: TWinControl;
     FControlList : {$ifdef IP_LAZARUS}TFPList{$else}TList{$endif};
     FCurURL : string;
     DoneLoading : Boolean;
@@ -2313,6 +2319,7 @@
     function getControlCount:integer;
     function getControl(i:integer):TIpHtmlNode;
   public
+    Body : TIpHtmlNodeBODY;
     constructor Create;
     destructor Destroy; override;
     function PagePtToScreen(const Pt: TPoint): TPoint;
@@ -2347,6 +2354,7 @@
     property PageViewBottom: Integer read FPageViewBottom;
     property PageViewTop: Integer read FPageViewTop;
     property ClientRect : TRect read FClientRect;
+    property Control: TWinControl read FControl;
     property ControlsCount: integer read getControlCount;
     property Controls[i:integer]: TIpHtmlNode read getControl;
     property FrameSet : TIpHtmlNodeFRAMESET read FCurFrameSet;
@@ -2375,8 +2383,6 @@
   end;
   {$ENDIF}
 
-  TIpHtmlInternalPanel = class;
-
   TIpHtmlScrollBar = class
   private
     FKind: TScrollBarKind;
@@ -12018,6 +12024,7 @@
   end;
 
 begin
+  inherited;
   Owner.ControlCreate(Self);
   aCanvas := TFriendPanel(Parent).Canvas;
   iCurFontSize := aCanvas.Font.Size;
@@ -12435,6 +12442,7 @@
   i, j, iCurFontSize: integer;
   OptGroup: TIpHtmlNodeOPTGROUP;
 begin
+  inherited;
   Owner.ControlCreate(Self);
   aCanvas := TFriendPanel(Parent).Canvas;
   iCurFontSize := aCanvas.Font.Size;
@@ -12599,6 +12607,7 @@
   iCurFontSize: integer;
   aCanvas : TCanvas;
 begin
+  inherited;
   Owner.ControlCreate(Self);
   aCanvas := TFriendPanel(Parent).Canvas;
   iCurFontSize := aCanvas.Font.Size;
@@ -13222,23 +13231,18 @@
 begin
   inherited Create(ParentNode);
   FElementName := 'button';
-  Owner.FControlList.Add(Self);
 end;
 
 destructor TIpHtmlNodeBUTTON.Destroy;
 begin
-  Owner.FControlList.Remove(Self);
   inherited;
 end;
 
 procedure TIpHtmlNodeBUTTON.CreateControl(Parent: TWinControl);
-var
-   iCurFontSize: integer;
-   aCanvas : TCanvas;
 begin
+  inherited;
   Owner.ControlCreate(Self);
-  aCanvas := TFriendPanel(Parent).Canvas;
-  iCurFontSize := aCanvas.Font.Size;
+
   FControl := TButton.Create(Parent);
   FControl.Visible := False;
   FControl.Parent := Parent;
@@ -13247,28 +13251,9 @@
   with TButton(FControl) do begin
     Enabled := not Self.Disabled;
     Caption := Value;
-    case ButtonType of
-    hbtSubmit :
-      begin
-        OnClick := SubmitClick;
-        if Caption = '' then
-          Caption := SHtmlDefSubmitCaption;
-      end;
-    hbtReset :
-      begin
-        OnClick := ResetClick;
-        if Caption = '' then
-          Caption := SHtmlDefResetCaption;
-      end;
-    hbtButton :
-      begin
-        OnClick := ButtonClick;
-      end;
-    end;
-    Width := TFriendPanel(Parent).Canvas.TextWidth(Caption) + 40;
-    Height := TFriendPanel(Parent).Canvas.TextHeight(Caption) + 10;
+    OnClick := ButtonClick;
   end;
-  aCanvas.Font.Size := iCurFontSize;
+  RecalcSize;
 end;
 
 procedure TIpHtmlNodeBUTTON.Reset;
@@ -13275,19 +13260,73 @@
 begin
 end;
 
-procedure TIpHtmlNodeBUTTON.ResetClick(Sender: TObject);
+procedure TIpHtmlNodeBUTTON.SetValue(AValue: string);
 begin
-  ResetRequest;
+  if FValue =AValue then Exit;
+  FValue :=AValue;
+  if Owner.DoneLoading and (FControl <> nil) then
+  begin
+    (FControl as TButton).Caption := AValue;
+    RecalcSize;
+  end;
 end;
 
-procedure TIpHtmlNodeBUTTON.SubmitClick(Sender: TObject);
+procedure TIpHtmlNodeBUTTON.RecalcSize;
+var
+   OldFontSize: integer;
+   aCanvas : TCanvas;
 begin
-  SubmitRequest;
+  with Control as TButton do
+  begin
+    aCanvas := TFriendPanel(Parent).Canvas;
+    OldFontSize := aCanvas.Font.Size;
+    Width := TFriendPanel(Parent).Canvas.TextWidth(Caption) + 40;
+    Height := TFriendPanel(Parent).Canvas.TextHeight(Caption) + 10;
+    aCanvas.Font.Size := OldFontSize;
+  end;
 end;
 
+procedure TIpHtmlNodeBUTTON.SetDisabled(AValue: Boolean);
+begin
+  if FDisabled =AValue then Exit;
+  FDisabled :=AValue;
+  if Owner.DoneLoading and (FControl <> nil) then
+    (FControl as TButton).Enabled := not AValue;
+end;
+
+procedure TIpHtmlNodeBUTTON.SetInputType(AValue: TIpHtmlButtonType);
+begin
+  //if FInputType =AValue then Exit; //nop we need it to set default caption
+  FInputType := AValue;
+  if Owner.DoneLoading and (FControl <> nil) then
+      if Self.Value = '' then //Value = Caption here
+        case FInputType of
+        hbtSubmit :
+           Self.Value := SHtmlDefSubmitCaption;
+        hbtReset :
+           Self.Value := SHtmlDefResetCaption;
+        hbtButton :
+          begin
+          end;
+        end;
+end;
+
 procedure TIpHtmlNodeBUTTON.ButtonClick(Sender: TObject);
 begin
-  Owner.ControlClick(Self);
+  case ButtonType of
+  hbtSubmit :
+    begin
+      SubmitRequest;
+    end;
+  hbtReset :
+    begin
+      ResetRequest;
+    end;
+  hbtButton :
+    begin
+      Owner.ControlClick(Self);
+    end;
+  end;
 end;
 
 function TIpHtmlNodeBUTTON.Successful: Boolean;
@@ -13510,6 +13549,10 @@
   inherited Create(ParentNode);
   Owner.FControlList.Add(Self);
   Align := hiaBottom;
+  if Owner.DoneLoading then
+  begin
+    CreateControl(Owner.Control);
+  end;
 end;
 
 destructor TIpHtmlNodeControl.Destroy;
@@ -15051,6 +15094,7 @@
     HyperPanel.OnHotClick := FViewer.HotClick;
     HyperPanel.OnClick := FViewer.ClientClick;
     HyperPanel.TabStop := FViewer.WantTabs;
+    FHtml.FControl := HyperPanel;
     FHtml.OnScroll := HyperPanel.ScrollRequest;
     FHtml.OnControlClick := ControlClick;
     FHtml.OnControlClick2 := ControlClick2;
iphtml.pas07.patch (8,016 bytes)   

wp

2020-10-21 23:18

developer   ~0126452

>> "What with the autosizing of the button when the caption is changed so that following html elements can be covered?"
> Already done in last patch.

You maybe do not understand what I mean. In the demo program there is a text element after the button ("Text following the button"). When the caption of the button is changed (by typing in the edit control) the width of the button changes, but the position of the text element does not change. I reverted my version back to your patch and it shows the same issue.

The resizing code for the button has another issue: When I add characters to the caption in the Edit, the width of the button increases - that's correct, but the left and right margins between button border and caption start/end grow too - I would not expect that because this is coded as the constant 40 in the code.

wp

2020-10-21 23:31

developer   ~0126453

Last edited: 2020-10-22 01:03

View 2 revisions

> I Just renamed HyperPanel to Control: TWinControl
I do not like this because one of the reasons why the IpHtml unit is so difficult to understand is that there are so many parts with similar names, so many panels, so many frames. Naming TIpHtml.HyperPanel as TIpHtml.Control makes the situation even worse: In TIpHtml the Control would be the LCL-Parent of the control nodes, in the control nodes the Control would be the LCL control itself.

> This is my last patch before I revert my changes to keep code as official
Thank you. I want to keep the AddNodeControl version which is in svn (until I find that something is absolutely woring). But it's good to have your last functional version here as a reference.

BTW, why is it necessary to call Owner.DoneLoading in the setters that you added to the button node?

Zaher Dirkey

2020-10-22 08:49

reporter   ~0126454

>BTW, why is it necessary to call Owner.DoneLoading

While loading, element node should not create control, it will be created in a loop in InternalCreateFrames,
but in creating on the fly, it is loaded and we can create it immediately

wp

2020-10-22 09:58

developer   ~0126455

Last edited: 2020-10-22 10:00

View 3 revisions

Sure. But I was asking about the setters: Why is it necesssary to check for Owner.DoneLoading when the caption of the button or the input type is changed at runtime? OK, it would prevent one unnecessary call to recalculate the button size. But any other reason?

-----------------

There is maybe a change to be applied to the AddNodeControl. In the current version, the button is added to the MasterFrame only. But a frame can also contain other frames accessible as IpHtmlPanel.MasterFrame.Frames[index]. In order to allow adding the run-time created button also to these frames I am thinking about extending the method to
procedure AddNodeControl(AControlNode: TIpHtmlNodeControl; AParentFrame: TIpHtmlFrame = nil)
where in case of AParentFrame=nil the MasterFrame will be used.

Is there any use case for this?

Pedro Gimeno

2020-10-22 13:23

reporter   ~0126456

I can't test yet, will do in the next days, but I wanted to add some notes in advance:

- I find it annoying that user code needs to care about implementation details such as whether a node has an associated control, and call AddNodeControl if so. It feels like something that should be transparent to the person adding nodes. That's why I like Zaher's approach better.
- In my use case, I will always fully configure the button before adding text after it. If, after a change of size caused by a larger caption, the text added afterwards is positioned correctly, my use case is solved.
- I agree with wp in that Control is a bad name for the HyperPanel.

> In the current version, the button is added to the MasterFrame only.

Oops. Wasn't that already solved in Zaher's patch by using the IpHtml of the frame where the control is?

wp

2020-10-22 17:03

developer   ~0126462

> Oops. Wasn't that already solved in Zaher's patch by using the IpHtml of the frame where the control is?

OK, I did not see that. This convinces me. I undid the AddNodeControl and introduced a TIpHtml.FParent (TWinContrl) according to Zaher's patch. I still call CreateControl in the constructor of TIpHtmlNodeBUTTON, but I think it should be called in its ancestor TIpHtmlNodeControl so that it is used by the other control types as well. However, when I modified the demo to run-time create a TIpHtmlNodeINPUT the edit control does not show up. So, something else is missing. but maybe this should go into another report.

Can I resolve this one?

Zaher Dirkey

2020-10-22 20:40

reporter   ~0126470

I prefer TIpHtml.FControl because this window is not parent of ipHTML, also more compatible with other element that have FControl too
sometime i spend days to find good name, it is part of documentation.

wp

2020-10-22 22:36

developer   ~0126475

But it is the parent of the Control of the TIpHtmlNodeControl. How about FControlParent?

Zaher Dirkey

2020-10-22 22:48

reporter   ~0126476

FControlParent is good

Pedro Gimeno

2020-10-24 23:26

reporter   ~0126524

I've updated to r64070 and it works perfectly, big thanks to wp and Zaher!

> Can I resolve this one?

Yes, I believe so. Thanks again.

wp

2020-10-25 00:09

developer   ~0126525

Please close after testing.

Issue History

Date Modified Username Field Change
2020-10-15 12:36 Pedro Gimeno New Issue
2020-10-15 12:41 Pedro Gimeno Note Added: 0126324
2020-10-17 17:10 Zaher Dirkey Tag Attached: ipHTMLPanel
2020-10-18 11:22 Zaher Dirkey Note Added: 0126386
2020-10-18 11:22 Zaher Dirkey File Added: iphtml.pas05.patch
2020-10-19 17:45 Pedro Gimeno Note Added: 0126417
2020-10-20 14:52 Zaher Dirkey Note Added: 0126423
2020-10-20 15:19 Zaher Dirkey Note Added: 0126424
2020-10-20 17:34 wp Note Added: 0126427
2020-10-20 17:35 wp Note Edited: 0126427 View Revisions
2020-10-20 17:35 wp Note Edited: 0126427 View Revisions
2020-10-20 17:37 wp Assigned To => wp
2020-10-20 17:37 wp Status new => assigned
2020-10-20 17:37 wp Status assigned => feedback
2020-10-20 17:37 wp LazTarget => -
2020-10-20 21:01 Zaher Dirkey Note Added: 0126430
2020-10-20 21:01 Zaher Dirkey File Added: iphtml.pas06.patch
2020-10-20 22:55 wp Note Added: 0126434
2020-10-20 22:58 Pedro Gimeno Note Added: 0126435
2020-10-20 22:58 Pedro Gimeno Status feedback => assigned
2020-10-21 11:28 Pedro Gimeno Note Added: 0126442
2020-10-21 20:09 wp Note Added: 0126445
2020-10-21 20:09 wp File Added: 37927 - IpHtmlPanel add button on the fly.zip
2020-10-21 20:09 wp Status assigned => feedback
2020-10-21 20:09 wp Note Edited: 0126445 View Revisions
2020-10-21 20:10 wp Note Edited: 0126445 View Revisions
2020-10-21 20:11 wp Note Edited: 0126445 View Revisions
2020-10-21 21:23 Zaher Dirkey Note Added: 0126447
2020-10-21 21:35 Zaher Dirkey Note Added: 0126448
2020-10-21 22:41 wp Note Added: 0126449
2020-10-21 22:57 Zaher Dirkey Note Added: 0126450
2020-10-21 23:11 Zaher Dirkey Note Added: 0126451
2020-10-21 23:11 Zaher Dirkey File Added: iphtml.pas07.patch
2020-10-21 23:18 wp Note Added: 0126452
2020-10-21 23:31 wp Note Added: 0126453
2020-10-22 01:03 wp Note Edited: 0126453 View Revisions
2020-10-22 08:49 Zaher Dirkey Note Added: 0126454
2020-10-22 09:58 wp Note Added: 0126455
2020-10-22 09:59 wp Note Edited: 0126455 View Revisions
2020-10-22 10:00 wp Note Edited: 0126455 View Revisions
2020-10-22 13:23 Pedro Gimeno Note Added: 0126456
2020-10-22 13:23 Pedro Gimeno Status feedback => assigned
2020-10-22 17:03 wp Note Added: 0126462
2020-10-22 17:03 wp Status assigned => feedback
2020-10-22 20:40 Zaher Dirkey Note Added: 0126470
2020-10-22 22:36 wp Note Added: 0126475
2020-10-22 22:48 Zaher Dirkey Note Added: 0126476
2020-10-23 12:17 wp Status feedback => resolved
2020-10-23 12:17 wp Resolution open => fixed
2020-10-23 12:17 wp Fixed in Revision => r64045, 64051, 64059
2020-10-23 12:17 wp LazTarget - => 2.2
2020-10-24 23:26 Pedro Gimeno Note Added: 0126524
2020-10-25 00:09 wp Note Added: 0126525