Menu

#559 Problems with the implementations of EM_SETMARGINS and EM_GETMARGINS

8
open
nobody
1
2023-12-25
2023-09-18
No

There are serious issues with the encapsulation of the EM_SETMARGINS and EM_GETMARGINS messages in TEdit and TRichEdit:

  1. The implementations of TEdit::SetLeftMargin and TEdit::SetRightMargin are plain wrong — the parameters to MkUint32 are switched, resulting in those calls always setting a margin of 0.
  2. The implementation of TEdit::SetMarginUseFontInfo is plain wrong — the documentation states that for edit controls the EC_USEFONTINFO must be passed in lParam.
  3. The functions are marked as "deleted" from TRichEdit implementation with a comment that they are not supported for rich edit controls, but the documentation states they are, even though EC_USEFONTINFO is handled differently.
  4. The function TEdit::GetMargins currently returns uint32, which the caller has to split into hi and low words. It may be more convenient if it returns a structure in which those are already split.
  5. TRichEdit::GetMargins is marked as "deleted" and the documentation states that EM_GETMARGINS is not supported by rich edit controls, but my tests indicate that it works and returns the correct values.

Note that issues 1) and 2) are carried over all the way from OWL 5.02, while 3) and 5) were introduced in OWLNext 6.04.

Suggestions:

  • Fix 1) and 2) in all LTS versions. Since the functions do not work correctly, most likely noone has ever used them, so no concerns about breaking existing code behavior.
  • In the trunk, revamp the API:
    1. Add a SetMargins function, since both margins can be set at the same time.
    2. Remove TEdit::SetMarginUseFontInfo and instead make the margin parameters to SetLeftMargin, SetRightMargin and SetMargins to default to EC_USEFONTINFO.
    3. Make GetMargins return a structure with left and right margin members.
    4. Remove the "deleted" functions for TRichEdit, including GetMargins as it seems to work correctly.
    5. Implement SetMarginUseFontInfo for TRichEdit, where it would set both the left and right margins.

Related

News: 2023/12/owlnext-7012-64422-6368-and-63015-updates
Wiki: OWLNext_Stable_Releases

Discussion

  • Vidar Hasfjord

    Vidar Hasfjord - 2023-09-20

    @jogybl wrote:

    Suggestions: Fix 1) and 2) in all LTS versions. [...]

    Agree, good catch! I trust your judgement on the other suggestions. And good to see you are preparing the branches for release. Full check-list for release can be found in [Contributing].

    PS. For 6.44, if you have the chance and energy to test it works with all supported compilers, consider the suggested C++17 compliance patch suggested by @lfastrup [bugs:#557].

     

    Related

    Bugs: #557
    Wiki: Contributing

  • Ognyan Chernokozhev

    The fixed SetMarginUseFontInfo function works well for Edit controls, but I cannot get it to work properly for Rich Edit controls

    If the same code as for Edit is used:

    SendMessage(EM_SETMARGINS, EC_LEFTMARGIN | EC_RIGHTMARGIN, MkUint32(EC_USEFONTINFO, EC_USEFONTINFO));
    

    it results in a blank (and seemingly very wide) rich edit control.

    While using the code that the documentation states should work:

    SendMessage(EM_SETMARGINS, EC_USEFONTINFO, 0)
    

    or even
    SendMessage(EM_SETMARGINS, EC_USEFONTINFO, MkUint32(EC_USEFONTINFO, EC_USEFONTINFO));
    have no effect - no margin is added to the rich edit control.

    On the other hand, the SetLeftMargin and SetRightMargin functions work fine for Rich Edit controls.

     
  • Vidar Hasfjord

    Vidar Hasfjord - 2023-12-25
    • labels: --> Internal, API, GUI
    • Description has changed:

    Diff:

    --- old
    +++ new
    @@ -1,7 +1,7 @@
     There are serious issues with the encapsulation of the [EM_SETMARGINS](https://learn.microsoft.com/en-us/windows/win32/controls/em-setmargins) and [EM_GETMARGINS](https://learn.microsoft.com/en-us/windows/win32/controls/em-getmargins) messages in TEdit and TRichEdit:
    
    -1. The implementations of TEdit::SetLeftMargin and TEdit::SetRightMargin are plain wrong - the parameters to MkUint32 are switched, resulting in those calls always setting a margin of 0.
    -2. The implementation of TEdit::SetMarginUseFontInfo is plain wrong - the documetation states that for edit controls the EC_USEFONTINFO must be passed in lParam.
    +1. The implementations of TEdit::SetLeftMargin and TEdit::SetRightMargin are plain wrong — the parameters to MkUint32 are switched, resulting in those calls always setting a margin of 0.
    +2. The implementation of TEdit::SetMarginUseFontInfo is plain wrong — the documentation states that for edit controls the EC_USEFONTINFO must be passed in lParam.
     3. The functions are marked as "deleted" from TRichEdit implementation with a comment that they are not supported for rich edit controls, but the documentation states they are, even though EC_USEFONTINFO is handled differently.
     4. The function TEdit::GetMargins currently returns uint32, which the caller has to split into hi and low words. It may be more convenient if it returns a structure in which those are already split.
     5. TRichEdit::GetMargins is marked as "deleted" and the documentation states that EM_GETMARGINS is not supported by rich edit controls, but my tests indicate that it works and returns the correct values.
    @@ -12,9 +12,9 @@
    
     * Fix 1) and 2) in all LTS versions. Since the functions do not work correctly, most likely noone has ever used them, so no concerns about breaking existing code behavior.
     * In the trunk, revamp the API:
    -1. Add a SetMargins function, since both margins can be set at the same time.
    -2. Remove TEdit::SetMarginUseFontInfo and instead make the margin parameters to SetLeftMargin, SetRightMargin and SetMargins to default to EC_USEFONTINFO.
    -3. Make GetMargins return a structure with left and right margin members.
    -4. Remove the "deleted" functions for TRichEdit, including GetMargins as it seems to work correctly.
    -5. Implement SetMarginUseFontInfo for TRichEdit, where it would set both the left and right margins.
    +    1. Add a SetMargins function, since both margins can be set at the same time.
    +    2. Remove TEdit::SetMarginUseFontInfo and instead make the margin parameters to SetLeftMargin, SetRightMargin and SetMargins to default to EC_USEFONTINFO.
    +    3. Make GetMargins return a structure with left and right margin members.
    +    4. Remove the "deleted" functions for TRichEdit, including GetMargins as it seems to work correctly.
    +    5. Implement SetMarginUseFontInfo for TRichEdit, where it would set both the left and right margins.
    
    • Group: unspecified --> 8
     
  • Vidar Hasfjord

    Vidar Hasfjord - 2023-12-25

    Issues 1 and 2 (implementation bugs in TEdit::SetLeftMargin, SetRightMargin and SetMarginUseFontInfo) have been fixed on the trunk [r6583], and the fixes have been merged into branches 6.30 [r6597], 6.36 [r6598], 6.44 [r6599] and 7.0 [r6600].

    I've retargeted the ticket to version 8 for the remaining API improvements.

     

    Related

    Commit: [r6583]
    Commit: [r6597]
    Commit: [r6598]
    Commit: [r6599]
    Commit: [r6600]


Log in to post a comment.