leaflet-elevation icon indicating copy to clipboard operation
leaflet-elevation copied to clipboard

Misalignment of chart profile when `preferCanvas: true` on MacOS

Open hupe13 opened this issue 2 years ago • 16 comments

Hi Raruto,

testing the pull request regarding gesture handling, I found out, that a similar SVG issue like this exists on MacOS Chrome 110 and Safari if:

preferCanvas = true

Chrome:

svg issue

Safari:

Bildschirmfoto 2023-02-22 um 12 10 07

(sorry about editing, I forgot to note my relavant setting).

hupe13 avatar Feb 22 '23 10:02 hupe13

Hi @hupe13,

so I guess the solution may be similar / related to: https://github.com/Raruto/leaflet-elevation/commit/b16c06ddd311bdd46f34a04989e0cb128c84aeae

In case you feel like doing some debugging, I suggest you to use browserstack local (if you don't already have an account 👉 https://www.browserstack.com/open-source)

👋 Raruto

Raruto avatar Feb 22 '23 16:02 Raruto

https://github.com/Raruto/leaflet-elevation/blob/868179234d8e9b8f7beaea57d0e34d8bd79a2622/src/components/d3.js#L613 This line must be

if(/Mac|iPod|iPhone|iPad/.test(navigator.platform) && /AppleWebkit/i.test(navigator.userAgent) && !/Chrome/.test(navigator.userAgent)) {

then Chrome works.

In Safari whithout any transformation console.log(canvas) shows, that canvas on the right position and size, but the diagram is to big and not in the canvas area. And the z-index is wrong.

if(/Mac|iPod|iPhone|iPad/.test(navigator.platform) && /AppleWebkit/i.test(navigator.userAgent) && !/Chrome/.test(navigator.userAgent)) {
       //canvas   .style("transform", "translate(" + margins.left + "px," + margins.top + "px)");
       console.log(canvas);
}

grafik

I can't get it in the right position.

hupe13 avatar Feb 23 '23 09:02 hupe13

@hupe13 Last night I did some local tests within the /test folder (with browserstack + MacOS Ventura) and it seemed to work correctly.

Do you have a particular test page of yours that is giving you problems? Perhaps are we testing different leaflet or leaflet-elevation versions?

👋 Raruto

Raruto avatar Feb 23 '23 09:02 Raruto

  • Macs with MacOS Catalina and MacOs Ventura
  • Maps with Leaflet v1.9.3 and leaflet-elevation v2.2.8 (actual).

You can see the problem on my page.

I have an error handler for the problem, it is currently turned off.

hupe13 avatar Feb 23 '23 10:02 hupe13

@hupe13 Did you test prefercanvas: true without wordpress as well?

From what I see it could also be that:

  • within the code a JS exception is thrown and that is not fully / correctly handled
  • there could be any <meta> tags or some css rules that interferes with computed screen pixel resolution (ref: d3js ?)

Below you can see how your test-mit-wp.gpx file looks to me if update the /test folder accordingly:

Mac OS Ventura + Chrome 110 + leaflet 1.9.3

image

Mac OS Ventura + Safari 16 + leaflet 1.9.3

image

👋 Raruto

Raruto avatar Feb 23 '23 15:02 Raruto

You are right, it is my problem, your example works. It could be that it is related to leaflet-ui, I don't use that. I am looking and will report.

Sorry and thank you for you help.

hupe13 avatar Feb 24 '23 06:02 hupe13

I have been able to narrow down the problem further but have not yet solved it. Your leaflet-elevation_extended-ui.html has this as well: Bildschirmfoto 2023-02-24 um 12 24 25

hupe13 avatar Feb 24 '23 11:02 hupe13

The reason was a css, I don't remember why I configured that. Probably updates from leaflet-elevation made it obsolete. I don't know. But the above problem still exists.

And you should change the line 613 (only this line) for Chrome:

if(/Mac|iPod|iPhone|iPad/.test(navigator.platform) && /AppleWebkit/i.test(navigator.userAgent) && !/Chrome/.test(navigator.userAgent)) {

hupe13 avatar Feb 24 '23 13:02 hupe13

The reason was a css, I don't remember why I configured that. Probably updates from leaflet-elevation made it obsolete. I don't know.

Don't keep me on my toes, i'm a curious guy! Which css rule? 🌵

And you should change the line 613 (only this line) for Chrome:

if(/Mac|iPod|iPhone|iPad/.test(navigator.platform) && /AppleWebkit/i.test(navigator.userAgent) && !/Chrome/.test(navigator.userAgent)) {

Ok, I'll let you know when I've done some testing

👋 Raruto

Raruto avatar Feb 24 '23 14:02 Raruto

I have been able to narrow down the problem further but have not yet solved it. Your leaflet-elevation_extended-ui.html has this as well:

Bildschirmfoto 2023-02-24 um 12 24 25

You should change the line 613 (only this line) for Chrome:

if(/Mac|iPod|iPhone|iPad/.test(navigator.platform) && /AppleWebkit/i.test(navigator.userAgent) && !/Chrome/.test(navigator.userAgent)) {

@hupe13 Am I missing something?, here's how I see it without changing anything in code:

image

Raruto avatar Feb 24 '23 17:02 Raruto

console.log(navigator.userAgent);

Safari logs:

Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.6.1 Safari/605.1.15

Chrome logs:

Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/110.0.0.0 Safari/537.36

leaflet-elevation_extended-ui.html is displayed correctly in Chrome without any changes. My example isn't.

I do not know what to do.

hupe13 avatar Feb 27 '23 14:02 hupe13

@hupe13 if you are referring to the z-index (chart tooltip) take a look at this too: Understanding z-index - The stacking context

If you link me again to the page that's giving you trouble, I'll look at it later too.

👋 Raruto

Raruto avatar Feb 27 '23 14:02 Raruto

@hupe13 ref: https://github.com/Raruto/leaflet-elevation/issues/232#issuecomment-1444050340

My fault, if you look closely at my image there is a horizontal shift in my example too.


I'm no expert on the subject but I imagine that by now the chrome developers have fixed the following bug:

  • https://stackoverflow.com/questions/63799631/svg-foreignobject-render-issue-in-chrome-and-safari
  • https://bugs.chromium.org/p/chromium/issues/detail?id=842668#c26

Anyway, I think the solution you proposed works correctly on Mac OS Chrome desktop, but it should also be tested on ios (chrome and mobile safari) before proceeding.

As for https://github.com/Raruto/leaflet-elevation/issues/232#issuecomment-1443554554, it seems me to be an issue related only to Safari browsers (if someone sends me a Mac I can also investigate better .. 😋).

👋 Raruto

Raruto avatar Feb 27 '23 17:02 Raruto

I don't have an iPhone also. I do not invest any more time at the moment, with the change of the line 613 and set preferCanvas to false on Safari it works fine on MacOS, and on Chrome also.

https://github.com/hupe13/extensions-leaflet-map-github/blob/4b3288ed2b3f9f1e21e7a5bb45d37f325057d2cf/php/elevation.php#L744-L749

hupe13 avatar Feb 28 '23 13:02 hupe13

@hupe13 continue here from https://github.com/Raruto/leaflet-elevation/pull/257#discussion_r1197201932

do you confirm that this is the patch you proposed in here? 👉 https://github.com/Raruto/leaflet-elevation/issues/232

Yes it works right now on Chrome and Safari.

But this problem still exists on Safari: grafik This is your /examples/leaflet-elevation.html. I set preferCanvas: true,.

Here's the reason behind this bug

  • https://bugs.webkit.org/show_bug.cgi?id=201110
  • https://output.jsbin.com/takesol
  • https://wpt.fyi/results/svg/extensibility/foreignObject/stacking-context.html?label=master&label=experimental&aligned

And here's how we might go about fixing it (not perfect, styles ? 🆘):

// Attempt to fix: https://bugs.webkit.org/show_bug.cgi?id=201110
function fixingWebkitBug201110({chart, panes, canvas, foreignObject}) {
  if (
    /Mac|iPod|iPhone|iPad/.test(navigator.platform) &&
    /AppleWebkit/i.test(navigator.userAgent) &&
    !/Chrome/.test(navigator.userAgent)
  ) {
    panes.tooltip.attr('id', 'webkit-' + Math.random().toFixed(20).slice(2));
    canvas.attr('style', canvas.attr('style')+ ';position: absolute; inset: 0; z-index:0;');
    foreignObject
    .append("svg:svg")
    .attr('style', 'position: absolute; inset: 0; width: 100%; height: 100%; z-index:1; stroke: #fff;')
    .html('<use href="#' + panes.tooltip.attr('id') + '" style="" />');
  }
}

// The following code is needed just to make you able to test it (without changing the source..)

// Hook "fixingWebkitBug201110" into class constructor
L.Control.Elevation.addInitHook(function() {
  this.on('elechart_init', function() {
    const chart = this._chart._chart;
    fixingWebkitBug201110({
      chart: chart,
      panes: chart.panes,
      canvas: chart.utils.canvas,
      foreignObject: chart.utils.canvas.select(function() { return this.parentNode; }
    });
  });
});

👋 Raruto

Raruto avatar May 19 '23 13:05 Raruto