View Issue Details

IDProjectCategoryView StatusLast Update
0037944PatchesLCLpublic2020-10-20 21:31
ReporterZaher Dirkey Assigned Towp  
PrioritynormalSeverityminorReproducibilityhave not tried
Status closedResolutionfixed 
Summary0037944: ipHTML Replace using -1 as undefined color to clNone
Descriptionin ipHTML they are using -1 as color that no defined to override it by other color element (parent, owner etc), I replaced -1 with clNone , it is more suite for the type of color, and more readable.
TagsNo tags attached.
Fixed in Revision64043
LazTarget2.2
Widgetset
Attached Files

Activities

Zaher Dirkey

2020-10-17 15:23

reporter  

turbopower_ipro04.patch (12,420 bytes)   
Index: components/turbopower_ipro/ipcss.inc
===================================================================
--- components/turbopower_ipro/ipcss.inc	(revision 64036)
+++ components/turbopower_ipro/ipcss.inc	(working copy)
@@ -839,8 +839,8 @@
 constructor TCSSProps.Create;
 begin
   FFont := TCSSFont.Create;
-  FBGColor := -1;
-  FColor := -1;
+  FBGColor := clNone;
+  FColor := clNone;
   FAlignment := haUnknown;
   FBorder := TCSSBorder.Create;
   FWidth.LengthType := cltUndefined;
@@ -989,8 +989,8 @@
 
 procedure TCSSProps.MergeAdditionalProps(AProps: TCSSProps);
 begin
-  if AProps.Color <> -1 then Color := AProps.Color;
-  if AProps.BGColor <> -1 then BGColor := AProps.BGColor;
+  if AProps.Color <> clNone then Color := AProps.Color;
+  if AProps.BGColor <> clNone then BGColor := AProps.BGColor;
   if AProps.Alignment <> haUnknown then Alignment := AProps.Alignment;
   if AProps.Font.Name <> '' then Font.Name := AProps.Font.Name;
   if AProps.Font.Size <> '' then Font.Size := AProps.Font.Size;
Index: components/turbopower_ipro/iphtml.pas
===================================================================
--- components/turbopower_ipro/iphtml.pas	(revision 64036)
+++ components/turbopower_ipro/iphtml.pas	(working copy)
@@ -4694,12 +4694,12 @@
   begin
     //DebugLn('MouseOver: ', classname);
     Props.DelayCache:=True;
-    if Props.HoverColor <> -1 then
+    if Props.HoverColor <> clNone then
     begin
       savedColor := Props.FontColor;
       Props.FontColor := Props.HoverColor;
     end;
-    if Props.HoverBgColor <> -1 then
+    if Props.HoverBgColor <> clNone then
     begin
       savedBgColor := Props.BgColor;
       Props.BgColor := Props.HoverBgColor;
@@ -4717,8 +4717,8 @@
   if IsMouseOver then
   begin
     Props.DelayCache:=True;
-    if Props.HoverColor <> -1 then Props.FontColor := savedColor;
-    if Props.HoverBgColor <> -1 then Props.BgColor := savedBgColor;
+    if Props.HoverColor <> clNone then Props.FontColor := savedColor;
+    if Props.HoverBgColor <> clNone then Props.BgColor := savedBgColor;
     Props.DelayCache:=False;
   end;
 end;
@@ -4778,9 +4778,9 @@
 begin
   inherited Create(ParentNode);
   FElementName := 'body';
-  FLink := -1;
-  FVLink := -1;
-  FALink := -1;
+  FLink := clNone;
+  FVLink := clNone;
+  FALink := clNone;
   Owner.Body := Self;
 end;
 
@@ -4796,7 +4796,7 @@
   end else begin
     {$IFDEF IP_LAZARUS}
     if BackGround = '' then begin
-      if BGColor <> -1 then begin
+      if BGColor <> clNone then begin
         Owner.Target.Brush.Color := BGColor;
         Owner.Target.FillRect(Owner.ClientRect);
       end else begin
@@ -4809,7 +4809,7 @@
       Owner.Target.Brush.Color := clWhite;
       Owner.Target.FillRect(Owner.ClientRect);
     end;
-    if BGColor <> -1 then begin
+    if BGColor <> clNone then begin
       Owner.Target.Brush.Color := BGColor;
       Owner.Target.FillRect(Owner.ClientRect);
     end;
@@ -4855,13 +4855,13 @@
   Props.DelayCache := True;
   inherited LoadAndApplyCSSProps;
   LinkProps := Owner.CSS.GetPropsObject('a:link', '');
-  if (LinkProps <> nil) and (LinkProps.Color <> -1) then
+  if (LinkProps <> nil) and (LinkProps.Color <> clNone) then
     Link := LinkProps.Color;
   LinkProps := Owner.CSS.GetPropsObject('a:visited', '');
-  if (LinkProps <> nil) and (LinkProps.Color <> -1) then
+  if (LinkProps <> nil) and (LinkProps.Color <> clNone) then
     VLink := LinkProps.Color;
   LinkProps := Owner.CSS.GetPropsObject('a:active', '');
-  if (LinkProps <> nil) and (LinkProps.Color <> -1) then
+  if (LinkProps <> nil) and (LinkProps.Color <> clNone) then
     ALink := LinkProps.Color;
   Props.DelayCache := True;
 end;
@@ -7620,7 +7620,7 @@
 var
   R, G, B, Err : Integer;
 begin
-  Result := -1;
+  Result := clNone;
   if S = '' then
     Exit;
   S := UpperCase(S);
@@ -7660,7 +7660,7 @@
       if FlagErrors then
         ReportError(SHtmlInvColor + S)
       else
-        Result := -1;
+        Result := clNone;
     end;
 end;
 
@@ -7953,7 +7953,7 @@
   FStartSel.x := -1;
   FEndSel.x := -1;
   //FixedTypeface := 'Courier New';
-  FBgColor := -1;
+  FBgColor := clNone;
   FFactBAParag := 1;
 end;
 
@@ -8188,15 +8188,15 @@
   DefaultProps.Preformatted := False;
   DefaultProps.NoBreak := False;
   if Body <> nil then begin
-    if Body.TextColor <> -1 then
+    if Body.TextColor <> clNone then
       DefaultProps.FontColor := Body.TextColor;
-    if Body.Link <> -1 then
+    if Body.Link <> clNone then
       DefaultProps.LinkColor := Body.Link;
-    if Body.VLink <> -1 then
+    if Body.VLink <> clNone then
       DefaultProps.VLinkColor := Body.VLink;
-    if Body.ALink <> -1 then
+    if Body.ALink <> clNone then
       DefaultProps.ALinkColor := Body.ALink;
-    if Body.BgColor <> -1 then
+    if Body.BgColor <> clNone then
       DefaultProps.BgColor := Body.BgColor;
   end;
 end;
@@ -9419,7 +9419,7 @@
         Props.FontSize := FONTSIZESVALUESARRAY[TmpSize-1];
     end;
   end;
-  if Color <> -1 then
+  if Color <> clNone then
     Props.FontColor := Color;
 end;
 
@@ -9513,8 +9513,8 @@
   LayouterClass: TIpHtmlBaseLayouterClass);
 begin
   inherited Create(ParentNode);
-  FBgColor := -1;
-  FTextColor := -1;
+  FBgColor := clNone;
+  FTextColor := clNone;
   FBackground := '';
   FLayouter := LayouterClass.Create(Self);
 end;
@@ -9574,9 +9574,9 @@
 begin
   inherited LoadAndApplyCSSProps;
   if FCombinedCSSProps <> nil then begin
-    if FCombinedCSSProps.Color <> -1 then
+    if FCombinedCSSProps.Color <> clNone then
       TextColor := FCombinedCSSProps.Color;
-    if FCombinedCSSProps.BgColor <> -1 then
+    if FCombinedCSSProps.BgColor <> clNone then
       BgColor := FCombinedCSSProps.BGColor;
   end;
 end;
@@ -10222,7 +10222,7 @@
 constructor TIpHtmlNodeHR.Create(ParentNode: TIpHtmlNode);
 begin
   inherited Create(ParentNode);
-  FColor := -1;
+  FColor := clNone;
   Align := hiaCenter;
   SizeWidth := TIpHtmlPixels.Create;
 end;
@@ -10244,10 +10244,10 @@
   R.Bottom := TopLeft.y + Dim.cy;
   if not PageRectToScreen(R, R) then
     Exit;
-  if NoShade or (Color <> -1) then begin
+  if NoShade or (Color <> clNone) then begin
     SavePenColor := aCanvas.Pen.Color;
     SaveBrushColor := aCanvas.Brush.Color;
-    if Color = -1 then begin
+    if Color = clNone then begin
       aCanvas.Pen.Color := clBlack;
       aCanvas.Brush.Color := clBlack;
     end else begin
@@ -10647,7 +10647,7 @@
 begin
   inherited Create(ParentNode);
   FElementName := 'table';
-  BgColor := -1;
+  BgColor := clNone;
   SizeWidth := TIpHtmlPixels.Create;
   SizeWidth.PixelsType := hpUndefined;
   FBorderColor := $808080;
@@ -10722,7 +10722,7 @@
   if (FOwner.Body.BgPicture <> nil) or (Props.BGColor = 1) then
     aCanvas.Brush.Style := bsClear
   else
-  if (Props.BGColor <> -1) and PageRectToScreen(BorderRect, R) then begin
+  if (Props.BGColor <> clNone) and PageRectToScreen(BorderRect, R) then begin
     aCanvas.Brush.Color :=Props.BGColor;
     aCanvas.FillRect(R);
   end;
@@ -11050,8 +11050,8 @@
   FElementName := 'tr';
   FAlign := haDefault;
   FValign := hvaMiddle;
-  FBgColor := -1;
-  FTextColor := -1;
+  FBgColor := clNone;
+  FTextColor := clNone;
 end;
 
 function TIpHtmlNodeTR.GetAlign: TIpHtmlAlign;
@@ -11066,7 +11066,7 @@
   if Assigned(FCombinedCSSProps) then begin
     if not (FCombinedCSSProps.Alignment in [haDefault, haUnknown]) then
       Align := FCombinedCSSProps.Alignment;
-    if FCombinedCSSProps.BgColor <> -1 then
+    if FCombinedCSSProps.BgColor <> clNone then
       BgColor := FCombinedCSSProps.BGColor;
     // wp: what about VAlign?
   end;
@@ -12194,13 +12194,13 @@
 {
   if Assigned(FInlineCSSProps) then
   begin
-       if FInlineCSSProps.BGColor <> -1 then FControl.Color := FInlineCSSProps.BGColor;
-       if FInlineCSSProps.Color <> -1 then FControl.Font.Color := FInlineCSSProps.Color;
+       if FInlineCSSProps.BGColor <> clNone then FControl.Color := FInlineCSSProps.BGColor;
+       if FInlineCSSProps.Color <> clNone then FControl.Font.Color := FInlineCSSProps.Color;
        if FInlineCSSProps.Font.Size <> '' then FControl.Font.size := GetFontSizeFromCSS(FControl.Font.size, FInlineCSSProps.Font.Size);
   end;
 }
   inherited;
-  if (Props.BgColor <> -1) and (
+  if (Props.BgColor <> clNone) and (
     (FControl is {$IFDEF VERSION3ONLY}TRadioButton{$ELSE}THtmlRadioButton{$ENDIF}) or
     (FControl is TCustomEdit)) then
     FControl.Color := Props.BgColor;
@@ -12829,9 +12829,9 @@
   if FHoverPropsRef <> nil then
   begin
     Props.DelayCache:=True;
-    if FHoverPropsRef.Color <> -1 then
+    if FHoverPropsRef.Color <> clNone then
       Props.HoverColor := FHoverPropsRef.Color;
-    if FHoverPropsRef.BgColor <> -1 then
+    if FHoverPropsRef.BgColor <> clNone then
       Props.HoverBgColor := FHoverPropsRef.BgColor;
     Props.DelayCache:=False;
   end;
@@ -12910,11 +12910,11 @@
   begin
     props.DelayCache:=True;
     {$WARNING Setting these font colors and name messes up the alignment for some reason}
-    if ACSSProps.Color <> -1 then begin
+    if ACSSProps.Color <> clNone then begin
       Props.FontColor := ACSSProps.Color;
     end;
 
-    if ACSSProps.BGColor <> -1 then begin
+    if ACSSProps.BGColor <> clNone then begin
       Props.BgColor := ACSSProps.BGColor;
     end;
 
@@ -13383,7 +13383,7 @@
   FColSpan := 1;
   FAlign := haDefault;
   FVAlign := hva3Middle;
-  BgColor := -1;
+  BgColor := clNone;
 end;
 
 destructor TIpHtmlNodeTableHeaderOrCell.Destroy;
@@ -13547,9 +13547,9 @@
    LoadAndApplyCSSProps;
    if (props.FontSize <> -1) then
      FControl.Font.Size:= Props.FontSize;
-     if Props.FontColor <> -1 then
+     if Props.FontColor <> clNone then
        FControl.Font.Color:= Props.FontColor;
-     if Props.BGColor <> -1 then
+     if Props.BGColor <> clNone then
        FControl.Brush.Color:= Props.BGColor;
    result := True;
    {$ENDIF}
Index: components/turbopower_ipro/iphtmlblocklayout.pas
===================================================================
--- components/turbopower_ipro/iphtmlblocklayout.pas	(revision 64036)
+++ components/turbopower_ipro/iphtmlblocklayout.pas	(working copy)
@@ -1314,7 +1314,7 @@
     FCanvas.brush.Color := clHighLight;
     FCanvas.FillRect(R);
   end
-  else if FCurProps.BgColor <> -1 then
+  else if FCurProps.BgColor <> clNone then
   begin
     FCanvas.brush.Style := bsSolid;
     FCanvas.brush.Color := FCurProps.BgColor;
@@ -1331,8 +1331,8 @@
 
   if aCurWord.Owner.ParentNode = aCurTabFocus then
     FCanvas.DrawFocusRect(R);
-  if FCanvas.Font.color = -1 then
-    FCanvas.Font.color := clBlack;
+  if FCanvas.Font.Color = clNone then
+    FCanvas.Font.Color := clBlack;
   FCanvas.Font.Quality := FOwner.Owner.FontQuality;
   {$ENDIF}
   if aCurWord.AnsiWord <> NAnchorChar then
@@ -1500,7 +1500,7 @@
     Props.VAlignment := FTableElemOwner.VAlign;
   end;
 
-  if FTableElemOwner.BgColor <> -1 then
+  if FTableElemOwner.BgColor <> clNone then
     Props.BgColor := FTableElemOwner.BgColor;
 
   inherited Layout(Props, TargetRect);
@@ -1537,7 +1537,7 @@
   FOwner.LoadAndApplyCSSProps;
   {$ENDIF}
 //DebugLn('td :', IntToStr(Integer(Props.Alignment)));
-  if FTableElemOwner.BgColor <> -1 then
+  if FTableElemOwner.BgColor <> clNone then
     Props.BgColor := FTableElemOwner.BgColor;
   if FTableElemOwner.Align <> haDefault then
     Props.Alignment := FTableElemOwner.Align
@@ -1558,7 +1558,7 @@
   {$ENDIF}
   if FOwner.PageRectToScreen(FTableElemOwner.PadRect, R) then
   begin
-    if (Props.BgColor <> -1) then
+    if (Props.BgColor <> clNone) then
     begin
       FIpHtml.Target.Brush.Color := Props.BGColor;
       FIpHtml.Target.FillRect(R);
Index: components/turbopower_ipro/iphtmlprop.pas
===================================================================
--- components/turbopower_ipro/iphtmlprop.pas	(revision 64036)
+++ components/turbopower_ipro/iphtmlprop.pas	(working copy)
@@ -325,9 +325,9 @@
 begin
   inherited Create;
   FOwner := AOwner;
-  FPropRec.BgColor := -1;
-  FPropRec.HoverColor := -1;
-  FPropRec.HoverBgColor := -1;
+  FPropRec.BgColor := clNone;
+  FPropRec.HoverColor := clNone;
+  FPropRec.HoverBgColor := clNone;
 end;
 
 destructor TIpHtmlPropB.Destroy;
@@ -368,7 +368,7 @@
   FPropA.IncUse;
   FPropB := FPropsBCache.FDummyB;
   FPropB.IncUse;
-  //BgColor := -1;
+  //BgColor := clNone;
 end;
 
 destructor TIpHtmlProps.Destroy;
turbopower_ipro04.patch (12,420 bytes)   

wp

2020-10-17 22:27

developer   ~0126377

Thanks for the patch. Applied. Please test and close if ok.

There is a situation, however, when we may end up in the same situation as in the other report with clBlack: The new version does not allow to set an opaque color to transparent (clNone), there are lots of instructions like

    if AProps.Color <> clNone then Color := AProps.Color

But I don't know if this case really can happen for IpHtmlPanel.

Zaher Dirkey

2020-10-17 23:08

reporter   ~0126381

it is easy to replaceing it again to any value, but keeping it -1 is wrong I though.
so if clNone is transparent color, maybe we can use clDefault instead, i am not sure.
but also clDefault have meaning like (default color is white).
It is belong to you btw. I can change it again :)

wp

2020-10-17 23:33

developer   ~0126382

Last edited: 2020-10-17 23:35

View 2 revisions

In fact, clNone = $FFFFFFFF = -1, so your patch did not change anything at all.

I thought about introducing a new color, e.g. clUndefined = $10000000, which does not interfere with the system colors which have $80 in the top byte, and "normal" colors have $00 there. But clNone must also be in the code to really signal transparent painting. It did not work, the unstyled text in the demo above disappeared. I did not want to go any further because maybe this case cannot be created with the limited features of the IpHtmlPanel at all, and maybe this is not worth the effort. But if you want to try yourself, go ahead, I'll be glad to accept your patch.

Zaher Dirkey

2020-10-18 11:09

reporter   ~0126385

yes, I agree,
now clNone is to make the code more readable, and belong to color type,
using clNone mean here it is not opaque to drawing text, and it make it faster drawing (the text) in fact, but calculating it if should be non opaque or not is hard and cause more bugs.

Issue History

Date Modified Username Field Change
2020-10-17 15:23 Zaher Dirkey New Issue
2020-10-17 15:23 Zaher Dirkey File Added: turbopower_ipro04.patch
2020-10-17 22:21 wp Assigned To => wp
2020-10-17 22:21 wp Status new => assigned
2020-10-17 22:27 wp Status assigned => resolved
2020-10-17 22:27 wp Resolution open => fixed
2020-10-17 22:27 wp Fixed in Revision => 64043
2020-10-17 22:27 wp LazTarget => 2.2
2020-10-17 22:27 wp Note Added: 0126377
2020-10-17 23:08 Zaher Dirkey Note Added: 0126381
2020-10-17 23:33 wp Note Added: 0126382
2020-10-17 23:35 wp Note Edited: 0126382 View Revisions
2020-10-18 11:09 Zaher Dirkey Note Added: 0126385
2020-10-20 21:31 Zaher Dirkey Status resolved => closed