Skip to content

Hyperlink access to chords/lyrics/mp3... from the chat window (#591) #592

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.

Sign up for GitHub

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

Merged
corrados merged 2 commits into from
Sep 20, 2020
Merged

Hyperlink access to chords/lyrics/mp3... from the chat window (#591) #592

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9b09681
added code provided by jc-Rosichini in #591
corrados Sep 15, 2020
b09c44f
update by jc-Rosichini (https://github.com/corrados/jamulus/issues/59…
corrados Sep 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 30 additions & 4 deletions src/chatdlg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,34 @@ void CChatDlg::OnClearChatHistory()
void CChatDlg::AddChatText ( QString strChatText )
{
// add new text in chat window
txvChatWindow->append ( strChatText );

// notify accessibility plugin that text has changed
QAccessible::updateAccessibility ( new QAccessibleValueChangeEvent ( txvChatWindow, strChatText ) );

// analyze strChatText to check if hyperlink (limit ourselves to https:://)
int indx_http = strChatText.indexOf("https://", 0);
if (indx_http != -1)
{
int indx_space = strChatText.indexOf(" ", indx_http);
if (indx_space==-1) indx_space=strChatText.length();

int cutslash = (strChatText.at(indx_space-1)=='/') ? 1:0; // drop "/" if last character of url text
Copy link
Copy Markdown
Collaborator

@pljones pljones Sep 15, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why? That trailing slash can cause different site behaviour (in "badly behaved" sites).

Copy link
Copy Markdown

@jc-Rosichini jc-Rosichini Sep 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, the trailing slash (if any) would prevent for instance Firefox to open the site
I noticed that when entering as an hyperlink https://www.google.com/ versus https://www.google.com, this is the reason of cutting this trailing slash.
This is not a real issue anyway since any invalid url will be rejected by the browser :-)

Copy link
Copy Markdown
Collaborator

@pljones pljones Sep 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firefox opens (from your post above) both equally well. If it's a defect in the QTextBrowser widget's URL handling, that's a different matter. (And another reason to take care with using it for active links.)

Copy link
Copy Markdown
Collaborator

@pljones pljones Sep 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really, just simplify it. If I enter something as simple as http://example.com/?key=value, your slash stripper fails. I'd dump all that code and just find the index of either "http://" or "https://" through to space and pass that string.


QString URL_name = strChatText.mid(indx_http,indx_space-indx_http-cutslash); // as entered by the user
QUrl URL = QUrl::fromUserInput(URL_name);
Copy link
Copy Markdown
Collaborator

@pljones pljones Sep 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. With the check for http/https, this should mean at least half-way sane values go in. If it comes back invalid, the check below then catches it. I'd still like the text more readable than "normal" so it catches the reader attention and is clearer.


QString new_strChatText;
if ( URL.isValid() )
{
new_strChatText.append( strChatText.left( indx_http )+"<a href=\""+URL.toString()+"\">"+URL_name+"</a> "+strChatText.right( strChatText.length()-indx_space ) );
}
else new_strChatText.append( strChatText ); // no change

txvChatWindow->append ( new_strChatText );
// notify accessibility plugin that text has changed
QAccessible::updateAccessibility ( new QAccessibleValueChangeEvent ( txvChatWindow, new_strChatText) );
}
else
{
txvChatWindow->append ( strChatText );
// notify accessibility plugin that text has changed
QAccessible::updateAccessibility ( new QAccessibleValueChangeEvent ( txvChatWindow, strChatText ) );
}
}
7 changes: 5 additions & 2 deletions src/chatdlgbase.ui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,14 @@
<bool>false</bool>
</property>
<property name="textInteractionFlags">
<set>Qt::LinksAccessibleByKeyboard|Qt::LinksAccessibleByMouse|Qt::NoTextInteraction|Qt::TextBrowserInteraction|Qt::TextSelectableByKeyboard|Qt::TextSelectableByMouse</set>
<set>Qt::LinksAccessibleByKeyboard|Qt::LinksAccessibleByMouse|Qt::TextBrowserInteraction|Qt::TextSelectableByKeyboard|Qt::TextSelectableByMouse</set>
</property>
<property name="openLinks">
<bool>false</bool>
<bool>true</bool>
</property>
<property name="openExternalLinks">
<bool>true</bool>
</property>
Copy link
Copy Markdown
Collaborator

@pljones pljones Sep 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be useful, if we retain the licence popup as a separate dialog, for these properties to be set there, too.

</widget>
</item>
<item>
Expand Down