Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(svg): svg mouse event doesn't work normally in Firefox when using shadow. #812

Merged
merged 2 commits into from
Sep 10, 2021

Conversation

plainheart
Copy link
Collaborator

@plainheart plainheart commented Sep 1, 2021

Fix apache/echarts#15646
Fix apache/echarts#15697

It seems the layerX/layerY is unexpected in the case where the chart uses SVG renderer and has shadow filter.

From the above picture, we can see that the values of layerX/layerY(unexpected) are not the same as offsetX/offsetY(expected) and don't look like the offset relative to the "closest positioned element". (I couldn't know the reason)

According to CANIUSE and the release note of Firefox, the offsetX and offsetY have been supported since desktop Firefox 39

Support for MouseEvent.offsetX and MouseEvent.offsetY have been added on desktop (bug 69787, but not on Firefox for Android or Firefox OS (they will be added in Firefox 43).

And they have been also supported by Firefox for Android and Firefox OS since v43

Support for MouseEvent.offsetX and MouseEvent.offsetY have been activated on Firefox for Android and Firefox OS (bug 1204841).

As mentioned above, I'd like to use offsetX/offsetY for modern Firefox.

A reproducible demo:

// Please use SVG renderer
option = {
  title: {
    text: '某站点用户访问来源',
    subtext: '纯属虚构',
    left: 'center'
  },
  tooltip: {
    trigger: 'item'
  },
  legend: {
    orient: 'vertical',
    left: 'left',
  },
  series: [
    {
      name: '访问来源',
      type: 'pie',
      radius: '50%',
      data: [
        {value: 1048, name: '搜索引擎'},
        {value: 735, name: '直接访问'},
        {value: 580, name: '邮件营销'},
        {value: 484, name: '联盟广告'},
        {value: 300, name: '视频广告'}
      ],
      emphasis: {
        itemStyle: {
          shadowBlur: 10,
          shadowOffsetX: 0,
          shadowColor: 'red'
        }
      }
    }
  ]
};

In addition, if we change to use offsetX/offsetY, it will make the feature mentiond in #794 possible.

@@ -67,13 +67,16 @@ export function clientToLocal(
// <https://bugs.jquery.com/ticket/8523#comment:14>
// BTW3, In ff, offsetX/offsetY is always 0.
else if (env.browser.firefox
// use offsetX/offsetY for Firefox >= 39
// PENDING: consider Firefox for Android and Firefox OS? >= 43
&& env.browser.version < '39'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose '39' should be a number here?

Copy link
Collaborator Author

@plainheart plainheart Sep 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if the version string in some earlier versions will probably be like 88.0.1, though I saw the version string in the navigator.userAgent is like 88.0 in most of the versions. (Tested it in Firefox 50, 70, 80, 92)

UA FULL VERSION

UPDATE

After reading these two threads, I guess it may be by design. So it should be okay if we change to use number 39.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation. Seems string comparison is better. Let's keep it.

@pissang
Copy link
Contributor

pissang commented Sep 10, 2021

I'm really impressed by how you research the specs and fix these corner cases. Again thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants