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

Misalignment of chart profile when preferCanvas: true on MacOS #232

Open
hupe13 opened this issue Feb 22, 2023 · 16 comments
Open

Misalignment of chart profile when preferCanvas: true on MacOS #232

hupe13 opened this issue Feb 22, 2023 · 16 comments

Comments

@hupe13
Copy link
Contributor

hupe13 commented Feb 22, 2023

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 hupe13 changed the title SVG Chrome MacOS preferCanvas Chrome MacOS Feb 22, 2023
@hupe13 hupe13 changed the title preferCanvas Chrome MacOS preferCanvas MacOS Feb 22, 2023
@Raruto
Copy link
Owner

Raruto commented Feb 22, 2023

Hi @hupe13,

so I guess the solution may be similar / related to: b16c06d

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

@hupe13
Copy link
Contributor Author

hupe13 commented Feb 23, 2023

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

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.

@Raruto
Copy link
Owner

Raruto commented Feb 23, 2023

@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

@hupe13
Copy link
Contributor Author

hupe13 commented Feb 23, 2023

  • 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.

@Raruto
Copy link
Owner

Raruto commented Feb 23, 2023

@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 Raruto changed the title preferCanvas MacOS Misalignment of chart profile when preferCanvas: true on MacOS Feb 23, 2023
@hupe13
Copy link
Contributor Author

hupe13 commented Feb 24, 2023

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
Copy link
Contributor Author

hupe13 commented Feb 24, 2023

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
Copy link
Contributor Author

hupe13 commented Feb 24, 2023

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)) {

@Raruto
Copy link
Owner

Raruto commented Feb 24, 2023

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

@hupe13
Copy link
Contributor Author

hupe13 commented Feb 24, 2023

@Raruto
Copy link
Owner

Raruto commented Feb 24, 2023

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

@hupe13
Copy link
Contributor Author

hupe13 commented Feb 27, 2023

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.

@Raruto
Copy link
Owner

Raruto commented Feb 27, 2023

@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
Copy link
Owner

Raruto commented Feb 27, 2023

@hupe13 ref: #232 (comment)

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:

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 #232 (comment), it seems me to be an issue related only to Safari browsers (if someone sends me a Mac I can also investigate better .. 😋).

👋 Raruto

@hupe13
Copy link
Contributor Author

hupe13 commented Feb 28, 2023

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

@Raruto
Copy link
Owner

Raruto commented May 19, 2023

@hupe13 continue here from #257 (comment)

do you confirm that this is the patch you proposed in here? 👉 #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

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 Raruto closed this as completed in 0a4596c Jul 5, 2023
@Raruto Raruto reopened this Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants