Share button label and tooltip are mixed up

RESOLVED FIXED in Firefox 36

Status

()

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: dao, Assigned: pduguet33, Mentored)

Tracking

Trunk
Firefox 36
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][lang=js])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

4 years ago
+++ 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

4 years ago
Created attachment 8511211 [details]
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.
Attachment #8511211 - Flags: review?(dao)
(Reporter)

Comment 2

4 years ago
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
Attachment #8511211 - Flags: review?(dao)
As of my first bug to start up, can u please assign me this bug.
(Reporter)

Comment 4

4 years ago
(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.
(Reporter)

Updated

4 years ago
Whiteboard: [good first bug] → [good first bug][lang=js]

Comment 5

4 years ago
This looks like fixed. I don't see the tooltip and label properties.
(Reporter)

Comment 6

4 years ago
(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.
(Assignee)

Comment 7

4 years ago
Created attachment 8522562 [details] [diff] [review]
Fix patch

Fixed with this patch. My first one so hoping I did correctly :)
Attachment #8522562 - Flags: review?(dao)
(Reporter)

Comment 8

4 years ago
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?
Attachment #8522562 - Flags: review?(dao) → feedback+
(Assignee)

Comment 9

4 years ago
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.
(Assignee)

Comment 10

4 years ago
Created attachment 8523008 [details] [diff] [review]
Patch with Unix line endings
Attachment #8522562 - Attachment is obsolete: true
Attachment #8523008 - Flags: review?(dao)
(Reporter)

Comment 11

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

Looks good, thanks!
Attachment #8523008 - Flags: review?(dao) → review+
(Reporter)

Updated

4 years ago
Assignee: nobody → pduguet33
Keywords: checkin-needed
(Assignee)

Comment 12

4 years ago
Is there any test to produce or run ? What are the next steps ?
https://hg.mozilla.org/integration/fx-team/rev/9433adad2df8
Keywords: checkin-needed
Whiteboard: [good first bug][lang=js] → [good first bug][lang=js][fixed-in-fx-team]
(Reporter)

Comment 14

4 years ago
(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.
(Reporter)

Updated

4 years ago
Attachment #8511211 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/9433adad2df8
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Whiteboard: [good first bug][lang=js][fixed-in-fx-team] → [good first bug][lang=js]
Target Milestone: --- → Firefox 36
You need to log in before you can comment on or make changes to this bug.