Closed
Bug 956586
Opened 10 years ago
Closed 10 years ago
tooltip modernization - FillInHTMLTooltip is deprecated
Categories
(Thunderbird :: Mail Window Front End, enhancement)
Thunderbird
Mail Window Front End
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 29.0
People
(Reporter: mkmelin, Assigned: mkmelin)
Details
Attachments
(1 file, 1 obsolete file)
7.60 KB,
patch
|
protz
:
review+
|
Details | Diff | Splinter Review |
The functionality in FillInHTMLTooltip is now in a binding (bug 480356), so we should drop that code. That makes e.g. HTML5 form validation messages work. Thunderbird had tests for the tooltips - added in bug 561854. The xlink one would now fail. I don't think it's necessarily the correct behavior either, so I've dropped that part of the test. protz: do you want to review this, as you wrote the tests originally in bug 561854
Attachment #8355900 -
Flags: review?(jonathan.protzenko)
Comment 1•10 years ago
|
||
Comment on attachment 8355900 [details] [diff] [review] proposed fix Just reviewed this, thanks for the ping. This looks like an excellent change (I'm always happy to see more Thunderbird-specific code go away!). I'm r-'ing because I think we should completely get rid of the FillInHTMLTooltip function: a quick MXR search <http://mxr.mozilla.org/comm-central/search?string=FillInHTMLTooltip&filter=^[^\0]*%24&tree=comm-central> shows that you replaced all the occurrences, if I'm not mistaken, so we might as well remove it completely.
Attachment #8355900 -
Flags: review?(jonathan.protzenko)
Attachment #8355900 -
Flags: review-
Attachment #8355900 -
Flags: feedback+
Comment 2•10 years ago
|
||
For the record, bug 956579 removes FillInHTMLTooltip in convbrowser.xml.
Assignee | ||
Comment 3•10 years ago
|
||
Hm, should I remove the test too? The test works, but what it actually tests is now toolkit functionality. At least the xul in html part is already tested in toolkit now.
Comment 4•10 years ago
|
||
I think it's ok to leave the test; it's reasonably small, and we've had a history of things breaking at random because of changes in mozilla-central. Let's keep us covered, since having this broken could go a long way unnoticed.
Assignee | ||
Comment 5•10 years ago
|
||
Removing FillInHTMLTooltip completely
Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Attachment #8357699 -
Flags: review?(jonathan.protzenko)
Assignee | ||
Updated•10 years ago
|
Attachment #8355900 -
Attachment is obsolete: true
Comment 6•10 years ago
|
||
Comment on attachment 8357699 [details] [diff] [review] proposed fix, v2 I haven't run the tests myself, but I trust you that this is not breaking the build. The patch looks just right, thanks!
Attachment #8357699 -
Flags: review?(jonathan.protzenko) → review+
Assignee | ||
Comment 7•10 years ago
|
||
https://hg.mozilla.org/comm-central/rev/f444666b101c -> FIXED
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 29.0
You need to log in
before you can comment on or make changes to this bug.
Description
•