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

Chrome Extension clashing with HiDPI canvas polyfill #1226

Closed
kodi opened this issue Mar 1, 2017 · 11 comments · Fixed by #2658
Closed

Chrome Extension clashing with HiDPI canvas polyfill #1226

kodi opened this issue Mar 1, 2017 · 11 comments · Fixed by #2658
Labels
bug fingerprinting Relating to (canvas) fingerprinting detection

Comments

@kodi
Copy link

kodi commented Mar 1, 2017

HiDPI canvas polyfill is really famous and great library for HiDPI retina canvas support (if you don't use it you get blured canvas on retina/HiDPI screens)

Here is the explanation of what lib does:
http://jondavidjohn.github.io/hidpi-canvas-polyfill/

Here is the issue on their github:
jondavidjohn/hidpi-canvas-polyfill#29

I know many people that use this in their production code, it would be such a shame for them to experience this just by having this great extension, such the Privacy Badger is.

Chrome Version: 56.0.2924.87 (64-bit)

Please advise!

@kodi kodi changed the title Chrome Extension clashing with HiDPI canvas polyfill [rMBP] Chrome Extension clashing with HiDPI canvas polyfill Mar 1, 2017
@kodi kodi changed the title [rMBP] Chrome Extension clashing with HiDPI canvas polyfill Chrome Extension clashing with HiDPI canvas polyfill Mar 1, 2017
@roblarsen
Copy link

I'd be interested in seeing a reduced test case of that one.

@kodi
Copy link
Author

kodi commented Mar 1, 2017

I'm not sure what's happening, but from what I can see, the first call to the fillText works, and second fails to go trough the pollyfill, and it executes "raw" yielding small text.

Quick search over this repo indeed gives results while searching for fillText and it is related to the canvas, but this is my first time watching at this code, and I don't have context of what this does and how its being used.

https://github.com/EFForg/privacybadger/search?utf8=%E2%9C%93&q=fillText

@kodi
Copy link
Author

kodi commented Mar 1, 2017

Also, bit context on the lib it self:

The canvas is blurry on the retina displays. (Ha, go figure :) )

To solve that, there is this pollyfill that scales the cavas 2x, and then also overrides all the native canvas methods and just multiplies x/y coords with the pixel ratio (in case of rMBP 2)

This gives sharp and clean graphics on the canvas.

To my knowledge, this was first covered in this, now famous article:
https://www.html5rocks.com/en/tutorials/canvas/hidpi/

And later, good people of the Internet created that pollyfill that you just include on the page and it does everything automagically 👍

@roblarsen
Copy link

@kodi What I meant was a simplified code example on some public resource (plunker or codepend or whatever the cool kids use nowadays) that showed it breaking. I'm just a passerby (I just started following this repo yesterday looking for an opportunity to maybe help out) so I know about as much as you do, but this a bug that looks like it might be interesting (to me at least.)

@kodi
Copy link
Author

kodi commented Mar 1, 2017

@roblarsen

Sure, here is really simplified usecase (I used modified index.html from their repo):

https://jsfiddle.net/kodisha/4xgjpgk6/

Expected results (works when I disable Privacy Badger):
image

Results with Privacy Badger ON:
image

@kodi
Copy link
Author

kodi commented Mar 1, 2017

btw, i noticed obvious text difference, not sure about other elements.

@roblarsen
Copy link

It is interesting. I can verify the bug.

@cooperq cooperq added the bug label Mar 1, 2017
@cowlicks
Copy link
Contributor

This seems like it is probably related to the fingerprinting monitoring. There is some canvas hacking in there. @roblarsen This would probably be a good place to start if you are interested https://github.com/EFForg/privacybadger/blob/master/src/fingerprinting.js#L154

@roblarsen

This comment has been minimized.

@ghostwords

This comment has been minimized.

@ghostwords ghostwords added the fingerprinting Relating to (canvas) fingerprinting detection label Feb 18, 2020
STRML added a commit to STRML/privacybadger that referenced this issue Jul 30, 2020
The method may be overridden again after we override,
as is done in the `hidpi-canvas-polyfill`. So if we
restore the method to the original as we see it, we'll
wipe out the polyfill.

Fixes EFForg#2657, EFForg#1226
@STRML
Copy link
Contributor

STRML commented Jul 30, 2020

Fixed in #2658, here's what's happening:

  • Privacy Badger loads, and overrides methods like CanvasRenderingContext2D.prototype.fillText to add monitoring. It saves a handle to the original method, orig.
  • hidpi-canvas-polyfill also loads, and overrides the same method, causing fillText to actually call the HiDPI method first, then Privacy Badger, then the original method
  • This optimization runs, which restores orig. But orig does not have the HiDPI override!
    if (is_canvas_write) {
    // optimization: one canvas write is enough,
    // restore original write method
    // to this CanvasRenderingContext2D object instance
    this[item.propName] = orig;
    }
  • The HiDPI code is lost, and incorrect rendering hits the screen as shown in Privacy Badger breaks text on Canvas charting software #2657.

The fix is to check if you've been overridden before restoring your method. If you have, you can do a very slightly more costly optimization, where the function simply bails out every time it runs thereafter.

STRML added a commit to STRML/privacybadger that referenced this issue Jul 30, 2020
The method may be overridden again after we override,
as is done in the `hidpi-canvas-polyfill`. So if we
restore the method to the original as we see it, we'll
wipe out the polyfill.

Fixes EFForg#2657, EFForg#1226
STRML added a commit to STRML/privacybadger that referenced this issue Jul 31, 2020
The method may be overridden again after we override,
as is done in the `hidpi-canvas-polyfill`. So if we
restore the method to the original as we see it, we'll
wipe out the polyfill.

Fixes EFForg#2657, EFForg#1226
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug fingerprinting Relating to (canvas) fingerprinting detection
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants