1088257 - Share button label and tooltip are mixed up

Status: RESOLVED FIXED

(Firefox :: Toolbars and Customization, defect)

Description

[Dão Gottwald [:dao]]

+++ This bug was initially created as a clone of Bug #1084419 +++

The share button uses social-share-button.label as the tooltip and social-share-button.tooltiptext as the label. This makes no sense.

We should remove the label and tooltiptext properties here, because they should be picked up automatically based on the id:

http://hg.mozilla.org/mozilla-central/annotate/d8de0d7e52e0/browser/components/customizableui/CustomizableWidgets.jsm#l396

Comment 1

[firefox]

Attachment: This is the updated file with the unnecessary lines (398, 399) of code removed.

This is the modified file that does not have the label or tooltip properties for the share button. I was not sure if this was a patch or what.

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 8511211 [details]
This is the updated file with the unnecessary lines (398, 399) of code removed.

You need to create the patch using "hg diff", e.g. "hg diff > ~/mypatch.diff". You can read more about it here: https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_diff_and_patch_files.3F

Before you create the patch, make sure Mercurial is set up properly:
https://developer.mozilla.org/en-US/docs/Installing_Mercurial#Basic_configuration

Comment 3

[Siddartha L K [:siddu]]

As of my first bug to start up, can u please assign me this bug.

Comment 4

[Dão Gottwald [:dao]]

(In reply to Siddartha L K [:siddu] from comment #3)
> As of my first bug to start up, can u please assign me this bug.

Please feel free to just attach a patch, we'll then formally assign the bug to you.

Comment 5

[tejdeepg]

This looks like fixed. I don't see the tooltip and label properties.

Comment 6

[Dão Gottwald [:dao]]

(In reply to tejdeepg from comment #5)
> This looks like fixed. I don't see the tooltip and label properties.

They're still there as far as I can tell.

Comment 7

[Paul Duguet]

Attachment: Fix patch

Fixed with this patch. My first one so hoping I did correctly :)

Comment 8

[Dão Gottwald [:dao]]

Comment on attachment 8522562 [details] [diff] [review]
Fix patch

Looks good, but this patch is using DOS line endings (\r\n) rather than Unix line endings (\n), which prevents it from applying cleanly. I'm not sure how this happened. Do you think you can fix it?

Comment 9

[Paul Duguet]

This is probably because I made it on ubuntu and transfered it to windows before submitting it. I'm going to fix that and work solely from the VM.

Comment 10

[Paul Duguet]

Attachment: Patch with Unix line endings

Comment 11

[Dão Gottwald [:dao]]

Comment on attachment 8523008 [details] [diff] [review]
Patch with Unix line endings

Looks good, thanks!

Comment 12

[Paul Duguet]

Is there any test to produce or run ? What are the next steps ?

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/9433adad2df8

Comment 14

[Dão Gottwald [:dao]]

(In reply to Paul Duguet from comment #12)
> Is there any test to produce or run ? What are the next steps ?

Unless your patch causes unexpected problems, we're done here and the fix will appear in one of the next nightly builds.

Comment 15

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/9433adad2df8