817579 - Command buttons in toolbox do not have tooltip

Status: RESOLVED FIXED

(DevTools :: General, defect, P2)

Description

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

Tilt/responsive view/scratchpad command buttons in toolbox don't have tooltip.

Comment 1

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

Attachment: patch v1

Comment 2

[Mihai Sucan [:msucan]]

Comment on attachment 687802 [details] [diff] [review]
patch v1

Review of attachment 687802 [details] [diff] [review]:
-----------------------------------------------------------------

Paul: are you working on a patch for this issue?

Tetsuharu: thank you for your bug report and patch. This is something Paul should review.

This patch doesn't seem to work on my system (Ubuntu). I still don't see tooltips.

Comment 3

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

(In reply to Mihai Sucan [:msucan] from comment #2)
> This patch doesn't seem to work on my system (Ubuntu). I still don't see
> tooltips.

I tried this patch on OSX, tooltiptext attribute is set correctly.
But the timing showing tooltip is severe. It may have other bug's effect...

Comment 4

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

Attachment: patch v2

> I tried this patch on OSX, tooltiptext attribute is set correctly.
> But the timing showing tooltip is severe. It may have other bug's effect...

Sorry. This is my mistake.
When I had tried the patch, it worked correctly. But when I have attached the patch v1 to bugzilla, I modified the patch unintentionally...


New patch v2 is working correctly. Please review it.

Comment 5

[Joe Walker [:jwalker]]

Comment on attachment 688238 [details] [diff] [review]
patch v2

Review of attachment 688238 [details] [diff] [review]:
-----------------------------------------------------------------

r+ from me, but could we in DeveloperToolbar.jsm, do

    if (command.tooltipText != null) {
      button.setAttribute("tooltiptext", command.tooltipText);
    }
    else if (command.description != null) {
      button.setAttribute("tooltiptext", command.description);
    }

Comment 6

[Joe Walker [:jwalker]]

Mihai, obviously don't let my r+ prevent you from reviewing if you want to.

Comment 7

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

Attachment: patch v3

This followed Joe's comment.

I ask Mihai to review this again tentatively.

Comment 8

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

Attachment: patch v3

This followed Joe's comment.

I ask Mihai to review this again tentatively.

Comment 9

[Ed Morley [:emorley]]

Comment on attachment 688702 [details] [diff] [review]
patch v3

(Double post of attachment)

Comment 10

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

(In reply to Ed Morley [UTC+0; email:edmorley@moco] from comment #9)
> Comment on attachment 688702 [details] [diff] [review]
> patch v3
> 
> (Double post of attachment)

Thank you, Ed.

Comment 11

[Mihai Sucan [:msucan]]

Comment on attachment 688704 [details] [diff] [review]
patch v3

This works for me now. Thank you Tetsuharu!

Also thanks Joe for looking into the patch. I wanted Paul or you to check this patch before I gave my r+.

Comment 12

[Mihai Sucan [:msucan]]

Landed in fx-team:

https://hg.mozilla.org/integration/fx-team/rev/2939067fa4e3

Comment 13

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

(In reply to Mihai Sucan [:msucan] from comment #12)
> Landed in fx-team:
> 
> https://hg.mozilla.org/integration/fx-team/rev/2939067fa4e3

Thank you, Mihai!

Comment 14

[Joe Walker [:jwalker]]

https://tbpl.mozilla.org/?rev=8a30e07815ff

Comment 15

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

(In reply to Joe Walker [:joe_walker] [:jwalker] from comment #14)
> https://tbpl.mozilla.org/?rev=8a30e07815ff

Joe, what happen?

Comment 16

[Tetsuharu OHZEKI [:tetsuharu] (UTC+9)]

https://hg.mozilla.org/mozilla-central/rev/2939067fa4e3