Closed Bug 1361180 Opened 3 years ago Closed 3 years ago

Graphics on Yahoo Finance not displaying properly

Categories

(Core :: Layout, defect)

53 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr45 --- unaffected
firefox-esr52 --- unaffected
firefox53 --- wontfix
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: jonhauke, Assigned: u459114)

References

Details

(Keywords: regression)

Attachments

(3 files)

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20170413192749

Steps to reproduce:

When I updated from Firefox 52.0.2 to 53.0, certain graphics on Yahoo Finance no longer work.  I frequently use technical indicators (such as EMA), and many of these indicators no longer plot properly.  When I revert back to Firefox 52.0.2 everything works properly.

To reproduce, go to Yahoo and look at the chart for a stock (e.g. https://finance.yahoo.com/chart/DEO).  Click "Add Indicator" and choose "Exponential Moving Average".  In Firefox 53.0 nothing appears on the chart.  In Firefox 52.0.2, a line showing the EMA appears properly.

This also happens for several other indicators I frequently use (MACD and RSI).


Actual results:

With Firefox 53.0, certain graphics do not plot correctly on Yahoo Finance.  These graphics plot correctly on Firefox 52.0.2.


Expected results:

Lines representing various technical indicators should be plotted on Yahoo's chart.  These lines properly appear on Firefox 52.0.2.
Component: Untriaged → Graphics
Product: Firefox → Core
Flags: needinfo?(cku)
See Also: → 1293929
Assignee: nobody → cku
Flags: needinfo?(cku)
Attachment #8863813 - Flags: review?(mstange)
Attachment #8863850 - Flags: review?(mstange)
Comment on attachment 8863813 [details]
Bug 1361180 - Part 1. Return error DrawResult to the caller when the mask url can not be resolved.

https://reviewboard.mozilla.org/r/135560/#review140258
Attachment #8863813 - Flags: review?(mstange) → review+
Comment on attachment 8863850 [details]
Bug 1361180 - Part 2. Reftest for unresolved mask in svg.

https://reviewboard.mozilla.org/r/135582/#review140260
Attachment #8863850 - Flags: review?(mstange) → review+
Pushed by cku@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7d6cfbbc07a4
Part 1. Return error DrawResult to the caller when the mask url can not be resolved. r=mstange
https://hg.mozilla.org/integration/autoland/rev/ce92201af9b7
Part 2. Reftest for unresolved mask in svg. r=mstange
https://hg.mozilla.org/mozilla-central/rev/7d6cfbbc07a4
https://hg.mozilla.org/mozilla-central/rev/ce92201af9b7
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Please request Beta approval on this when you get a chance.
Flags: needinfo?(cku)
Flags: in-testsuite+
Attached patch Part 2 for betaSplinter Review
Flags: needinfo?(cku)
Comment on attachment 8863813 [details]
Bug 1361180 - Part 1. Return error DrawResult to the caller when the mask url can not be resolved.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1319667
[User impact if declined]:trend line on Yahoo Finance may disappear
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]:yes
[Needs manual test from QE? If yes, steps to reproduce]:no 
[List of other uplifts needed for the feature/fix]: this patch, Part 1, and "Part 2 for beta"
[Is the change risky?]:no
[Why is the change risky/not risky?]: pass auto/manual test, the logic changed in the patch is simple.
[String changes made/needed]: N/A
Attachment #8863813 - Flags: approval-mozilla-beta?
Comment on attachment 8863813 [details]
Bug 1361180 - Part 1. Return error DrawResult to the caller when the mask url can not be resolved.

Fix an UI issue and include test. Beta54+. Should be in 54 beta 7.
Attachment #8863813 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to C.J. Ku[:cjku](UTC+8) from comment #16)
> [Is this code covered by automated tests?]: yes
> [Has the fix been verified in Nightly?]:yes
> [Needs manual test from QE? If yes, steps to reproduce]:no 

Setting qe-verify- based on C.J. Ku's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.