Patch #31441

Show elements titles using jQuery UI tooltips

Added by Marius BALTEANU 3 months ago. Updated about 2 hours ago.

Status:ReopenedStart date:
Priority:NormalDue date:
Assignee:Go MAEDA% Done:

0%

Category:UI
Target version:4.1.0

Description

Browser default title tooltip is quite non friendly for the user and/or UI development. For example, you cannot control the delay and HTML is not supported. Moving to jQuery UI Tooltip improves the current UI experience and will allow us in the future to make new improvements.

Regarding my proposed patch, I decided to not add an arrow to the tooltip because I don't think that it worth it.

date.png (23.1 KB) Marius BALTEANU, 2019-05-24 11:34

author.png (21.5 KB) Marius BALTEANU, 2019-05-24 11:34

tooltip.patch Magnifier (1.26 KB) Marius BALTEANU, 2019-05-24 11:38

tooltip_v2.patch Magnifier (1.17 KB) Marius BALTEANU, 2019-06-19 10:01

issue_subject_tooltip.png (50.7 KB) Marius BALTEANU, 2019-08-04 22:57

selectors_for_tooltip.patch Magnifier (1.76 KB) Marius BALTEANU, 2019-08-04 23:00

tooltip-for-issues@2x.png (11.1 KB) Go MAEDA, 2019-08-07 02:18

Associated revisions

Revision 18260
Added by Go MAEDA about 1 month ago

Show elements titles using jQuery UI tooltips (#31441).

Patch by Marius BALTEANU.

History

#1 Updated by Go MAEDA 3 months ago

  • Target version set to Candidate for next major release

+1 remarkably nice improvement!

#2 Updated by Go MAEDA 3 months ago

  • Target version changed from Candidate for next major release to 4.1.0

Setting the target version to 4.1.0.

#3 Updated by Mizuki ISHIKAWA 2 months ago

+1

#4 Updated by Seiei Miyagi 2 months ago

I found the XSS in attached patch.
If I fill the project description like below, then open http://localhost:3000/admin/projects and hover the title of the project, the alert is showing.

<script>alert(1)</script>

the content function needs to escape the $(this).prop('title')

#5 Updated by Marius BALTEANU 2 months ago

  • Assignee set to Marius BALTEANU

Seiei Miyagi wrote:

I found the XSS in attached patch.
If I fill the project description like below, then open http://localhost:3000/admin/projects and hover the title of the project, the alert is showing.

[...]

the content function needs to escape the $(this).prop('title')

You're right, thanks for pointing this out, I'll fix it.

#6 Updated by Marius BALTEANU about 1 month ago

I think for now it's enough to disable the HTML support (attached patch).

What do you think, Seiei Miyagi?

#7 Updated by Seiei Miyagi about 1 month ago

Thank you! It looks good to me.

Since the title attribute is too general and XSS may occur, I think it would be nice to disable HTML support.
IMO If you need HTML support, it may be better to add explicit attributes. (e.g. like html-safe? IDK)

#8 Updated by Go MAEDA about 1 month ago

  • Status changed from New to Closed
  • Assignee changed from Marius BALTEANU to Go MAEDA

Thank you to everyone who involved in this patch.

#9 Updated by Marius BALTEANU 13 days ago

I've just observed some bad effects of my patch. For example, in the gantt chart, the tooltip is shown even if the issue subject is visible:

I propose to replace the current [title] selector (which is too generic) with:
- a custom attribute data-toggle="tooltip" for enable the tooltip on elements
- '.icon-only' selector to target all the links rendered only as icon

#10 Updated by Go MAEDA 11 days ago

Marius BALTEANU wrote:

I propose to replace the current [title] selector (which is too generic) with:
- a custom attribute data-toggle="tooltip" for enable the tooltip on elements
- '.icon-only' selector to target all the links rendered only as icon

Tooltip for issues or some other objects will disappear after applying the new patch. Please see the screenshot below for an example. It is a pity if I cannot see the useful information any longer.

#11 Updated by Marius BALTEANU 10 days ago

Go MAEDA wrote:

Tooltip for issues or some other objects will disappear after applying the new patch. Please see the screenshot below for an example. It is a pity if I cannot see the useful information any longer.

You're right, but the plan is to add to all other objects that should have tooltips one by one. I'll update the patch to include the tooltip for object links as well.

#12 Updated by Marius BALTEANU about 2 hours ago

Go Maeda, is it better if we remove the title the from gantt issue subject?

Mariuss-MacBook-Pro:redmine mariusbalteanu$ git diff
diff --git a/lib/redmine/helpers/gantt.rb b/lib/redmine/helpers/gantt.rb
index ef9475cea..ed2a8f8f7 100644
--- a/lib/redmine/helpers/gantt.rb
+++ b/lib/redmine/helpers/gantt.rb
@@ -747,7 +747,6 @@ module Redmine
         when Issue
           tag_options[:id] = "issue-#{object.id}" 
           tag_options[:class] = "issue-subject hascontextmenu" 
-          tag_options[:title] = object.subject
           children = object.children & project_issues(object.project)
           has_children = children.present? && (children.collect(&:fixed_version).uniq & [object.fixed_version]).present?
         when Version

Also available in: Atom PDF