Patch #31441

Show elements titles using jQuery UI tooltips

Added by Marius BALTEANU about 1 month ago. Updated 7 days ago.

Status:ClosedStart 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

Associated revisions

Revision 18260
Added by Go MAEDA 7 days ago

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

Patch by Marius BALTEANU.

History

#1 Updated by Go MAEDA about 1 month ago

  • Target version set to Candidate for next major release

+1 remarkably nice improvement!

#2 Updated by Go MAEDA about 1 month 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 8 days ago

+1

#4 Updated by Seiei Miyagi 8 days 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 8 days 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 7 days 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 7 days 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 7 days ago

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

Thank you to everyone who involved in this patch.

Also available in: Atom PDF