1459905 - Left and right arrow navigation doesn't work if the SVG is rendered with browser native svg

Status: VERIFIED FIXED

(Core :: SVG, defect)

Description

[Oana Botisan, Desktop QA]

[Affected versions]:
- latest Nightly 62.0a1 (2018-05-08)
- Beta 61.0b3 (20180507191226)
- Release 59.0.3 (20180427210249)

[Affected platforms]:
- Windows 10 x64
- Mac OS X 10.11
- Ubuntu 16.04 x64

[Steps to reproduce]:
1. Launch Firefox.
2. Access the following website: http://codinginparadise.org/projects/svgweb/samples/demo.html?name=photos&svg.render.forceflash=false
3. Click on the left "<" and right buttons ">", below the "Show Demos Known to Fail" text.

[Expected result]:
- The photo is moved accordingly.

[Actual result]:
- Nothing happens when clicking on the right and left buttons.

[Regression range]:
- This seems to be an old regression, I was not able to reproduce it on FF 4.0, I will follow up with a regression range as soon as possible.

[Additional notes]:
- note that this issue happens only if the SVG is rendered with browser native svg
- not reproducible on Chrome or Edge.

Comment 1

[Alice0775 White]

Regression window:
m-central: https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d8be4bc4fba8&tochange=712eca11a04e
m-aurora: https://hg.mozilla.org/releases/mozilla-aurora/pushloghtml?fromchange=331b0a480586&tochange=91d5f962e553

Regressed by: Bug 821955

Comment 2

[ovidiu boca[:Ovidiu]]

I tested this on Window 10 with FF Nightly 67.0a1(2019-03-12), Beta 66.0b14, Release 65.0.2 and I can reproduce this issue.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: testcase 1 (ctrl+click to view as a SVG Document, in its own tab): purple rect should be equal distance from left edge as from the top edge

Comment 4

[Tim Spurway [:tspurway]]

this is no longer a regression

Comment 5

[Daniel Holbert [:dholbert]]

(In reply to Tim Spurway [:tspurway] from comment #4)

this is no longer a regression

Could you clarify what you mean by this? Per comment 1, we have a specific regression range identified. (This is a defect that was introduced by Bug 821955.)

Comment 6

[Daniel Holbert [:dholbert]]

I can see why release-management folks might not care to track it as a regression, since it broke a long time ago. But from a writing-the-patch perspective, it's extremely useful to have this labeled correctly as a regression, since it gives a strong hint about what code is broken [the code in the regressing bug's patch].

(I know we've got a "Regressed By" field coming soon, and maybe the semantics of the regression keyword will change when that field arrives [not sure]. But it's not here yet.)

Comment 7

[Daniel Holbert [:dholbert]]

Anyway - for now, I'm restoring the 'regression' keyword since it's still applicable per my understanding of what the keyword represents.

Comment 8

[Tim Spurway [:tspurway]]

Hey Daniel - yeah, thanks for restoring it. I was just trying to clear it off the stack of things we are tracking from a release mgmt perspective. But you're right, it's a regression.

Comment 9

[Daniel Holbert [:dholbert]]

(In reply to Tim Spurway [:tspurway] from comment #8)

Hey Daniel - yeah, thanks for restoring it. I was just trying to clear it off the stack of things we are tracking from a release mgmt perspective.

Thanks.

In cases like this, I'd suggest that you instead clear the "status-firefoxN" flags, since presumably those are part of what's getting it onto your "look into this for the upcoming release" dashboard, and those flags are only really useful/meaningful for bugs that regressed recently, not 45+ releases ago.

--> clearing those flags now.

Comment 10

[Daniel Kolsis]

Reproduced on latest Nightly 73.0a1 (10.12.2019) on Windows 7.

Comment 11

[Florencia Di Ciocco, NI to vbalducci]

I tested this on Ubuntu 18.04 with FF Nightly 73.0a1 (2019-12-10) (64-bit), and I can reproduce this issue.

Comment 12

[Augusto Pace]

This issue is still reproducible on FF Nightly 75.0a1 (2020-02-18) on Mac OS 10.14.

Comment 13

[Seba S.]

Hi,

Could reproduce this issue on Nightly 77.0a1 Win10 x64.

Regards,

Comment 14

[Virginia (please send need info request to Brindusa Tot)]

This issue is still reproducible on FF Nightly 80.0a1 32-bit on Windows 10

Comment 15

[Robert Longson [:longsonr]]

Attachment: Bug 1459905 - Merge DOMSVGTranslatePoint and DOMSVGPoint and fix updates

SVGSVGElement::SetCurrentScaleTranslate checks that things have changed but if we
manage to update both translate values before we get here then we'll skip the screen update
that we need.

Also

• introduces a tear off for SVGSVGElement.currentTranslate so we hand out the same
  object as required by the SVG idl
• removes SVGSVGElement::SetCurrentTranslate as dead code
• removes mPreviousScale and mPreviousTranslate from SVGSVGElement as they are no longer
  necessary

Comment 16

[Robert Longson [:longsonr]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ddb7a87e149550db3b19b465fd3cc3dabd048835

Comment 17

[Marcela (Please NI request to Brindusa Tot)]

Reproducible on latest Nigthly 81.0a1 (2020-08-19) on macOS 10.14.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6e2a9067912f
Merge DOMSVGTranslatePoint and DOMSVGPoint and fix updates r=emilio

Comment 19

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/6e2a9067912f

Comment 20

[Florencia Di Ciocco, NI to vbalducci]

Hi,

I tested this today on Windows 10 on Firefox Nightly 82.0a1 (2020-08-27) (64-bit) and the bug is still present. Should I have tested it tomorrow?

Regards, Flor.

Comment 21

[Daniel Holbert [:dholbert]]

Thanks, Florencia!

Indeed, it looks like the original STR are still broken, so there's something else going on here. It looks like the reduced testcase is fixed, though (for me at least), presumably fixed by the patch that landed here -- so for simplicity's sake, let's keep this bug closed and I've spun off a new one to track whatever's still broken about the original site / STR: bug 1661509

Comment 22

[Daniel Holbert [:dholbert]]

So: for verification purposes, please uses the reduced testcase (attachment 9051432 [details]) rather than the original site.

• Old/buggy results are for the purple rect to be against the left edge of the page,
• Expected results are for the purple rect to be offset some distance (about the same distance) from the left vs. top of the page.

Comment 23

[Florencia Di Ciocco, NI to vbalducci]

Hi Daniel,

Thanks for your reply and all of the details. I agree with you, the reduced test case does work and is solved on the latest nightly version. I'll add myself to cc on the new bug to provide any kind of test that you may need in the future (and that I can provide ;D ).

Regards, Flor.

Comment 24

[Florencia Di Ciocco, NI to vbalducci]

Hi Daniel and all,

I proceed to advance this bug too since bug 1661509 has been fixed. I hope this is ok.

Regards, Flor.