Open Bug 1446011 Opened 6 years ago Updated 17 days ago

Make getCTM and getScreenCTM compatible with other browsers

Categories

(Core :: SVG, enhancement, P3)

enhancement

Tracking

()

Webcompat Priority P2

People

(Reporter: jwatt, Unassigned)

References

(Depends on 1 open bug)

Details

(4 keywords, Whiteboard: [layout:backlog:quality])

The behavior of getCTM and getScreenCTM in other browsers is fairly useless in certain scenarios, but we've basically lost the battle to get them to change to our more useful interpretation of the spec. We should align with that behavior for sake of web compatibility.
I'm not sure whether it's this or the fact that we ignore CSS transforms (bug 972041 and bug 1073207) that's the bigger problem.
[Triage 2018/03/23 - P3]
Priority: -- → P3
As for all getScreenCTM() related bugs (and there are currently three of them!), there is a simple-but-hard-to-pass test, that would check for a reasonable implementation: 

If in abitrary DOM tree (ideally HTML and SVG mixed up) nodes are removed and reattached to a different parent, and a CSS or SVG transform is applied to it that accounts for the getScreentCTM() matrix of the source and destination parent (multiply the source matrix by the inverse of the destination matrix), the visual appearance, and the getScreenCTM() values of the node itself on the old and the new parent must be equal up to rounding errors.

While the visual apperance may change due to visual clipping, the getScreenCTM always should be equal. 

Applies to: #972041, #1073207, #1446011

(In reply to Jonathan Watt [:jwatt] from comment #0)

The behavior of getCTM and getScreenCTM in other browsers is fairly useless
in certain scenarios, but we've basically lost the battle to get them to
change to our more useful interpretation of the spec. We should align with
that behavior for sake of web compatibility.

Relating to getCTM, I believe the battle on the interpretation of the wording in the spec, in particular that of the nearestViewportElement, was always a lost cause. In the Bug 873106 discussion, Robert says:

"nearestViewportElement
The element which established the current viewport. Often, the nearest ancestor ‘svg’ element. Null if the current element is the outermost svg element.

So the nearestViewportElement is null

SVGMatrix getCTM()
Returns the transformation matrix from current user units (i.e., after application of the ‘transform’ attribute, if any) to the viewport coordinate system for the nearestViewportElement.

So we need to return the transformation matrix to the viewport coordinate system for something which is null"

The last sentence is wrong. "Null" is the nearest ancestor, where it doesn't exist.

The (unnecessary) sentence "Null if the current element is the outermost svg element" means "in the case of the outermost svg element, the nearestViewportElement is null", certainly not the outermost svg element, for which, yes, you can and must return the transformation matrix to the viewport coordinate system.

"Null [nearestViewportElement] if the current element is the outermost svg element" can not refer to the current element, since its descendant status is under investigation, and therefore does exist.

Independently of the interpretation, if we agree that in the case of the outermost svg element the nearestViewportElement is null, which we do, that doesn't justify to ignore the concrete element under analysis (the outermost svg element when it comes to it at the end of the chain). Therefore, add and stop, not just stop. This workaround pseudo code yields the same result of getCTM().e in all other implementations:

while (nearestViewportElement != null) {
x += nearestViewportElement.x
}

As things stand, the only way to skip the FF test whenever needed in applications and libraries, would be to just keep the workaround and discard getCTM(). That would be a shame, wouldn't it?

No matter what, to align seems the only sensible solution, and the sooner the better :)

Webcompat Priority: --- → ?
Whiteboard: [layout:triage-discuss]
Severity: normal → S3
Whiteboard: [layout:triage-discuss] → [layout:backlog:quality]

Hello all,

this issue is still persistent, especially with D3 and libraries that depend on it.

To demonstrate, I've created a minimal example: https://codepen.io/lucavazz/pen/abpPVoq
In this, the mouse position on hover is retrieved via d3.pointer (called d3.mouse before D3 v6) for two variants, both with a CSS transform of translate applied to their respective parent element.
While the lower div element works fine both in Firefox and Chrome, the upper svg based element does not work as expected in Firefox.

D3 internally uses getScreenCTM for the pointer function: https://github.com/d3/d3-selection/blob/v2.0.0/src/pointer.js#L11
This problem is known to D3, but no fix is intended as D3 does not want to implement workarounds for implementation differences across browsers (see https://github.com/d3/d3/issues/2810#issuecomment-213786022).

Hopefully this example helps with investigation :)

While there exist workarounds for this, it gets hard to implement them if a used library (i.e. Plotly https://www.npmjs.com/package/plotly.js) depends on D3.
To demonstrate, consider this example: https://codepen.io/lucavazz/pen/mdOpgMY
Hovering over Germany works for all four maps in Chrome, while in Firefox it only works for the top left one.

Considering that D3 is widely used (~4,000 dependents on npm alone: https://www.npmjs.com/package/d3) I'd argue that this should be treated as a defect in Firefox, not just as an enhancement.
With this additional information in mind, could you please re-consider the triage type and priority?

Flags: needinfo?(jwatt)
Flags: needinfo?(sean)

Possible duplicate of bug 1610093 ?

That's certainly the main issue that's left now, but this is a meta bug that's supposed to cover everything we find. The issues in comment 1 are both fixed now for instance.

Webcompat Priority: ? → P2

Clear a needinfo that is pending on an inactive user.

Inactive users most likely will not respond; if the missing information is essential and cannot be collected another way, the bug maybe should be closed as INCOMPLETE.

For more information, please visit auto_nag documentation.

Flags: needinfo?(sean)
Flags: needinfo?(jwatt)
Depends on: 1889781
No longer blocks: 1610093
Depends on: 1610093
Depends on: 1890198
You need to log in before you can comment on or make changes to this bug.