Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RichTextBox LinkClicked event doesn't work for "friendly named" hyperlinks #1631

Closed
RussKie opened this issue Aug 14, 2019 · 16 comments
Closed
Assignees
Labels
🪲 bug Product bug (most likely) 💥 regression-preview Regression from a preview release tenet-compatibility Incompatibility with previous versions or with WinForms for .NET Framework

Comments

@RussKie
Copy link
Member

RussKie commented Aug 14, 2019

  • .NET Core Version: (e.g. 3.0 Preview1, or daily build number, use dotnet --info)
  • Have you experienced this same bug with .NET Framework?: Yes

Problem description:

With the removal of the quirk Switch.System.Windows.Forms.DoNotLoadLatestRichEditControl (3f2ca8a) we may regress because we will use newer RTB controls (4.0+) and users will no longer be able to load older RTB controls that handled some hyperlinks differently.

Please refer to https://stackoverflow.com/q/56936777/2338036

Actual behavior:

Expected behavior:

Minimal repro:

@weltkante
Copy link
Contributor

Title might need a fix? The linked article talks about a regression in hyperlinks, not about images.

users will no longer be able to load older RTB controls that handled some hyperlinks differently

The linked article suggests that only as a workaround for a regression in WinForms, not because the RTB handles hyperlinks differently. RTB 5.0 has a "friendly name hyperlink" feature which WinForms apparently doesn't understand.

So while your argument that downgrading to older RTB controls is no longer possible is true, for the linked problem the "right thing to do" would be to fix the bug in WinForms, not having people work around it by using older RTB versions.

@r-aghaei
Copy link

@RussKie The linked SO question is about LinkClicked event.

I also tried editing an image (PBrush object class) in RichTextBox which worked as expected in both .NET Core and .NET Framework.

@RussKie
Copy link
Member Author

RussKie commented Aug 14, 2019

Yes, thank you - looked at the wrong title in the original ticket.

@RussKie RussKie changed the title RichTextBox can't edit image which attached to RTF file RichTextBox LinkClicked event doesn't work for "friendly named" hyperlinks Aug 14, 2019
@RussKie RussKie self-assigned this Aug 14, 2019
@RussKie RussKie added this to the 3.0.0-GA milestone Aug 14, 2019
@zsd4yr
Copy link
Contributor

zsd4yr commented Aug 16, 2019

@RussKie @weltkante It sounds to me like we now use RTB 5.0 and that there is additional work to support functionality new to 5.0 that we have not wrapped appropriately; does that sound right?

@zsd4yr zsd4yr added the 🪲 bug Product bug (most likely) label Aug 16, 2019
@weltkante
Copy link
Contributor

weltkante commented Aug 16, 2019

@zsd4yr Yes, thats what the linked stackoverflow article says in the response marked as answer, it's pretty detailed in outlining the problem in the WinForms implementation leading to events not firing. I'll quote a very reduced version here for quick summary but the stackoverflow response linked in the OP is worth a read:

The source of the problem appears to be a hack in the original RichTextBox.CharRangeToString Method

When using the Friendly Name Hyperlinks available in the RichEdit50 control, the RichTextBox.Text.Length property can be less than the c.cpMax value as the link is not included in the returned property value. This causes the method to return String.Empty to the calling RichTextBox.EnLinkMsgHandler Method that in turn will not raise the LickClicked event if a Empty.String is returned.

@RussKie
Copy link
Member Author

RussKie commented Aug 16, 2019

Right now we have a broader issue of link clicked events not working at all - #1644

@weltkante
Copy link
Contributor

#1644 works when initialization is moved from constructor to load event, so its not related to hyperlinks, it seems to be a general pattern that in .NET Core there's a problem when things are initialized in the constructor

@dotnet dotnet deleted a comment from weltkante Aug 19, 2019
@RussKie

This comment has been minimized.

@RussKie RussKie marked this as a duplicate of #1520 Aug 20, 2019
@RussKie RussKie closed this as completed Aug 20, 2019
@weltkante
Copy link
Contributor

weltkante commented Aug 20, 2019

@RussKie #1520 is very unclearly formulated, the title does not match with the screenshot, so its not clear why this should be a duplicate. It may very well be that the zip repro attached to #1520 also repros this problem, but that doesn't make #1520 a duplicate if the issue describes something different. If you keep the multiple bugs of #1520 in one issue you'll have to make sure all get fixed before closing.

@RussKie
Copy link
Member Author

RussKie commented Aug 20, 2019

We've discussed it with our testers, who have submitted #1520, and it was resolved as reporting the same issue.
The screenshot is misleading indeed, and it threw few people off the right track.

@weltkante
Copy link
Contributor

weltkante commented Aug 20, 2019

@RussKie considering you said in PR #1661 that Issue #1520 will be fixed by said PR, this issue can't be a duplicate.

This repros on Desktop Framework and I'd assume it also repros in .NET Core. Since the bug fixed by PR #1661 never existed in Desktop Framework it can't be the same issue. The problem the stackoverflow article was about is a regression in the Desktop Framework long ago (by updating to a newer version of the RTF control which supported features WinForms doesn't understand).

I'd recommend to reopen so this can be fixed at some point. Of course you are free to reprioritize, it might not be necessary to fix for 3.0 since it repros on Desktop Framework by default and the only thing you took away is the known workaround to downgrade the RTF control.

(Note for anyone wanting to investigate: the exact location of the bug was already root-caused in the stackoverflow article linked in the issue description, so you don't need to do that yourself again.)

using System;
using System.Drawing;
using System.Windows.Forms;

public class Form1 : Form
{
    [STAThread]
    static void Main()
    {
        Application.EnableVisualStyles();
        Application.SetCompatibleTextRenderingDefault(false);
        Application.Run(new Form1());
    }

    private RichTextBox richTextBox1;

    public Form1()
    {
        richTextBox1 = new RichTextBox();
        richTextBox1.Dock = DockStyle.Fill;
        richTextBox1.LinkClicked += richTextBox1_LinkClicked;
        ClientSize = new Size(640, 480);
        Controls.Add(richTextBox1);
        Load += Form1_Load;
    }

    private void Form1_Load(object sender, EventArgs e)
    {
        richTextBox1.Rtf = @"{\rtf1\ansi\ansicpg1252\deff0\nouicompat\deflang4105{\fonttbl{\f0\fnil\fcharset0 Calibri;}}
{\*\generator Riched20 10.0.17134}\viewkind4\uc1 
{\field{\*\fldinst { HYPERLINK ""http://www.google.com"" }}{\fldrslt {Click here}}}
\pard\sa200\sl276\slmult1\f0\fs22\lang9  for more information.\par
}";
    }

    private void richTextBox1_LinkClicked(object sender, LinkClickedEventArgs e)
    {
        MessageBox.Show($"link {e.LinkText} clicked");
    }
}

@RussKie
Copy link
Member Author

RussKie commented Aug 20, 2019

@weltkante thank you for being persistent.
I have run this scenario locally and, you're right, it is still broken.

@RussKie RussKie reopened this Aug 20, 2019
@RussKie RussKie marked this as not a duplicate of #1520 Aug 20, 2019
@merriemcgaw merriemcgaw added 💥 regression-preview Regression from a preview release tenet-compatibility Incompatibility with previous versions or with WinForms for .NET Framework labels Aug 21, 2019
@RussKie
Copy link
Member Author

RussKie commented Aug 22, 2019

@JeremyKuhne @Tanya-Solyanik can you think of any implications if we remove the following check?

if (c.cpMax > Text.Length || c.cpMax - c.cpMin <= 0)
{
return string.Empty;
}

I have run few tests with few different rtfs and everything seemed to work.

This check was presumably added in response to "VSWhidbey 504502" bug, but I don't know where to find it.

@RussKie RussKie added the 🚧 work in progress Work that is current in progress label Sep 2, 2019
RussKie added a commit to RussKie/winforms that referenced this issue Sep 3, 2019
The newer RichEdit controls (v4.1+) in specific cases had troubles
detecting "friendly named" hyperlinks as the `Text` property was
reportig less characters than the rage of the actual hyperlink.

Fixes dotnet#1631
Relates to dotnet#1139
@ghost ghost removed the 🚧 work in progress Work that is current in progress label Sep 4, 2019
@RussKie RussKie closed this as completed Sep 4, 2019
@RussKie RussKie removed this from the 3.0-GA milestone Sep 4, 2019
@Marie-Shi
Copy link

Verified this bug with .Net core 3.0.100-rc1-014190 from release branch, RichTextBox LinkClicked event can work for "friendly named" hyperlinks. Please see the following Gif:
RichTextBox_LinkClicked

@liquidboy
Copy link

liquidboy commented May 21, 2020

just like to chime in and say how much of a pain this is, just encountered it in 4.8 desktop winforms app (we are building plugins for Autodesk navisworks/revit)

@merriemcgaw
Copy link
Member

Have you tried adding the setting which reverts back to the older RTB in .NET 4.8?
<runtime> <AppContextSwitchOverrides value="Switch.System.Windows.Forms.DoNotLoadLatestRichEditControl=true" /> </runtime>

If this is a significant problem that the apps config file I would suggest you use VS Feedback to report the problem and tell us how this is impacting the Autodesk app in particular. We can look into what can be done. Though to set expectations, .NET Framework 4.8 is in servicing only mode and the bar for fixes is quite high. It would be good to get this reported though so we can keep track of the impact.

@ghost ghost locked as resolved and limited conversation to collaborators Feb 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🪲 bug Product bug (most likely) 💥 regression-preview Regression from a preview release tenet-compatibility Incompatibility with previous versions or with WinForms for .NET Framework
Projects
None yet
Development

No branches or pull requests

7 participants