flutter / devtools

Performance tools for Flutter
https://flutter.dev/docs/development/tools/devtools/
BSD 3-Clause "New" or "Revised" License
1.58k stars 326 forks source link

Widget Inspector values overlap label text in Brave browser #367

Closed DanTup closed 5 years ago

DanTup commented 5 years ago

Opened by @Sh1d0w at https://github.com/Dart-Code/Dart-Code/issues/1487


Just updated to the latest version 2.24 and this is how it looks when I launch the Dart DevTools:

screenshot 2019-02-27 at 11 12 10

The widget attributes seems to overflow the widget name and everything becomes unreadable.

Sh1d0w commented 5 years ago

After a short investigation it seems to be caused by the browser's built in ad-blocker. It prevents third party fingerprinting on the canvas element used, which seems to break the UI. After disabling this, everything looks good.

This issue can be closed I guess, thanks for the fast reaction đź‘Ť

DanTup commented 5 years ago

caused by the browser's built in ad-blocker

Is this enabled by default? Which browser/version is it on which OS? It may be worth keeping this open to see if there's a way we can fix it, or at least warn users if they're using that browser.

Sh1d0w commented 5 years ago

@DanTup I am on the latest version of MacOS using Brave Browser. Yes this is enabled by default.

jacob314 commented 5 years ago

To keep our velocity high writing the devtools app we only have time to support Chrome. We should make that clearer when you start the app to avoid confusion.

DanTup commented 5 years ago

We should make that clearer when you start the app to avoid confusion.

Do you mean in the web app? Should we open an issue for this?

I had a quick look to see if there might be a quick fix for this, but it seems like Brave's anti-fingerprinting breaks quite a lot of stuff that uses canvas:

https://github.com/brave/brave-browser/issues/1025

So I doubt there's something simple we could tweak to avoid it. I guess a lot of legit use looks like fingerprinting.

kenzieschmoll commented 5 years ago

Open issue for warning users is here: https://github.com/flutter/devtools/issues/66.

lookfirst commented 5 years ago

Super new Flutter user. Just ran into this myself. Having a warning for missing canvas would be nice.

image

DanTup commented 5 years ago

@lookfirst Brave deliberately makes itself look like Chrome (it used to identify itself, but stopped because it was another variable that could be used for fingerprinting), so I don't think it's possible to show a warning for Brace without all Chrome users seeing it.

Sh1d0w commented 5 years ago

I don’t think the warning should be based on browser detection, rather than if canvas is available. There are plugins from chrome that prevents fingerprinting as well, which then will result broken UI again.

DanTup commented 5 years ago

Oh sorry, I may have misunderstood. I thought Brave was just messing with the rendering on the canvas. If it's just not available, then that might be easy to warn for.

Can you run this code in the JS console and confirm if it outputs false?

var elm = document.createElement('canvas'); !!(elm.getContext && elm.getContext('2d'));
Sh1d0w commented 5 years ago

@DanTup That's close.

Canvas fingerprinting is well described here https://browserleaks.com/canvas#how-does-it-work

Because the way it works, the only way to prevent it is to disable toDataUrl method of the canvas, so the data can not be read, and thus no "fingerprint" can be obtained.

I think you can tweak the check like this:

var elm = document.createElement('canvas'); (elm.toDataURL() === '')

If fingerprinting is disabled, this will output true.

DanTup commented 5 years ago

Ok, though we don't use toDataURL in DevTools so it might not be the best check. I wonder if it's measureText that's being blocked (looks like we use that to align the value, and the screenshot shows the value written over the label).

Does this return 0 for you?

document.createElement('canvas').getContext('2d').measureText('Text that is quite long')
Sh1d0w commented 5 years ago

@DanTup Yep, the output is

TextMetrics {width: 0}

Good point.

I've just ran observatory with the latest version of Brave, as I wasn't using it for a while and found out a bunch of new problems:

Screenshot 2019-07-09 at 11 29 25

It seems like the CSS and JS assets are not served with correct mime type from the observatory thus white screen appears as those files are not loaded at all.

DanTup commented 5 years ago

@Sh1d0w ok, then I think we can probably easily show a warning at the top of the Inspector page when measureString returns 0. I've opened https://github.com/flutter/devtools/issues/777.

As for the Observatory issue - I think that's best raised at dart-lang/sdk since it's a different app to the Dart DevTools (though I'm not sure what the support is intended to be for that).

alexis-andino commented 4 years ago
image

I turned off the shields for the website and the overlap issue was gone.