View Issue Details

IDProjectCategoryView StatusLast Update
0034886LazarusLCLpublic2019-11-04 10:25
ReporterKenneth HippoliteAssigned ToJuha Manninen 
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Product Version1.8.4Product Build 
Target VersionFixed in Version 
Summary0034886: PageControl does not instantiate PageClass
Descriptionlcl\include\pagecontrol.inc, line 190

function TPageControl.AddTabSheet: TTabSheet;
begin
  Result := TTabSheet.Create(Self);

incorrect. Should be:

  Result := TTabSheet(PageClass.Create(Self));
Additional InformationWithout this change, even if you set PageClass to a custom descendant, you'll get TTabSheets instead. Oddly, you can force-cast them into your custom descendant but they really aren't and cannot pass the "X is TMyDescendant" test
TagsNo tags attached.
Fixed in Revisionr62136
LazTarget-
Widgetset
Attached Files

Relationships

related to 0036227 closedJuha Manninen [regression] Building anchordocking fails 

Activities

Martin Friebe

2019-01-17 20:03

manager   ~0113450

Generally not opposed to the idea. But...

In order to make that change some safeguards would be needed (IMHO).

The main problem is, that currently PageClass might not inherit from TTabSheet. Yet in TPageControl exactly that must be enforced.


This may be achieve-able. Yet before making changes the current implementation would need to be examined towards the intent that led to it. That is an svn blame may have to be conducted.


PageClass really should not be a variable in protected visibility. It should be a property.

- A virtual setter could then verify the required inheritance.
- A getter (if any) must not be allowed to be virtual. Otherwise it could deliver an unverified class as result.

Note:
- "A getter" would be different from the "provider for the default value" which is GetPageClass (the typical name for the getter).
- A getter is also not required, direct access to the field would be ok. But should be guarded by a comment, that if ever it was made a getter it must not be a virtual one.

With the getter always getting the result that was set by the setter, the setter can not be skipped (In order to set "strict private" FPageClass all parent setters must be called/traversed). So the check will be guaranteed.


Further more, for compile time checks, TPageControl could re-introduce the property with a correct "class of TTabSheet" type.
This can be bypassed by type-casting, but then the setter would catch it.

Re-introductions will not be seen by parent classes, so they can not be used to bypass the class verification.

Juha Manninen

2019-10-27 20:24

developer   ~0118879

I eliminated the whole variable and used GetPageClass instead everywhere.
AddTabSheet now uses a safe typecast which reveals a wrong type at runtime.
  Result := GetPageClass.Create(Self) as TTabSheet;

Please test.

Pascal Riekenberg

2019-10-29 10:36

reporter   ~0118909

This breaks build of Anchordocking!

anchordocking.pas(7622,3) Error: Identifier not found "PageClass"

Issue History

Date Modified Username Field Change
2019-01-16 21:10 Kenneth Hippolite New Issue
2019-01-17 20:03 Martin Friebe Note Added: 0113450
2019-10-27 20:15 Juha Manninen Assigned To => Juha Manninen
2019-10-27 20:15 Juha Manninen Status new => assigned
2019-10-27 20:24 Juha Manninen Status assigned => resolved
2019-10-27 20:24 Juha Manninen Resolution open => fixed
2019-10-27 20:24 Juha Manninen Fixed in Revision => r62136
2019-10-27 20:24 Juha Manninen LazTarget => -
2019-10-27 20:24 Juha Manninen Note Added: 0118879
2019-10-29 10:36 Pascal Riekenberg Note Added: 0118909
2019-10-29 13:46 Maxim Ganetsky Relationship added related to 0036227