Skip to content

Commit d97c43b

Browse files
authored
[Bug] Fix Region mode being off on high dpi screens like Retina or Surface Book in Firefox (#305)
* Use a devicePixelRatio of 1 on HDPI screens on Firefox to work around a bug in their canvas.drawImage(...) implementation * Add logging and make it work for zoom * Remove parameter for high dpi displays
1 parent ecdb6f6 commit d97c43b

File tree

2 files changed

+19
-6
lines changed

2 files changed

+19
-6
lines changed

src/scripts/clipperUI/regionSelector.tsx

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {DomUtils} from "../domParsers/domUtils";
33
import * as Log from "../logging/log";
44

55
import {Constants} from "../constants";
6+
import {ClientType} from "../clientType";
67
import {Status} from "./status";
78

89
import {ClipperStateProp} from "./clipperState";
@@ -199,8 +200,17 @@ class RegionSelectorClass extends ComponentBase<RegionSelectorState, ClipperStat
199200
* the maximum allowed size)
200201
*/
201202
private startRegionClip() {
203+
// Taken from https://www.kirupa.com/html5/detecting_retina_high_dpi.htm
204+
// We check this here so that we can log it as a custom property on the regionSelectionProcessingEvent
205+
const query = "(-webkit-min-device-pixel-ratio: 2), (min-device-pixel-ratio: 2), (min-resolution: 192dpi)";
206+
const isHighDpiScreen = matchMedia(query).matches;
207+
const isFirefoxWithHighDpiDisplay = this.props.clipperState.clientInfo.clipperType === ClientType.FirefoxExtension && isHighDpiScreen;
208+
202209
// Firefox reports this value incorrectly if this iframe is hidden, so store it now since we know we're visible
203-
this.devicePixelRatio = window.devicePixelRatio;
210+
// In addition to this, Firefox currently has a bug where they are not using devicePixelRatio correctly
211+
// on HighDPI screens such as Retina screens or the Surface Pro 4
212+
// Bug link: https://bugzilla.mozilla.org/show_bug.cgi?id=1278507
213+
this.devicePixelRatio = isFirefoxWithHighDpiDisplay ? window.devicePixelRatio / 2 : window.devicePixelRatio;
204214

205215
let regionSelectionProcessingEvent = new Log.Event.BaseEvent(Log.Event.Label.RegionSelectionProcessing);
206216
let regionSelectionCapturingEvent = new Log.Event.BaseEvent(Log.Event.Label.RegionSelectionCapturing);
@@ -213,6 +223,7 @@ class RegionSelectorClass extends ComponentBase<RegionSelectorState, ClipperStat
213223
this.saveCompressedSelectionToState(dataUrl).then((canvas) => {
214224
regionSelectionProcessingEvent.setCustomProperty(Log.PropertyName.Custom.Width, canvas.width);
215225
regionSelectionProcessingEvent.setCustomProperty(Log.PropertyName.Custom.Height, canvas.height);
226+
regionSelectionProcessingEvent.setCustomProperty(Log.PropertyName.Custom.IsHighDpiScreen, isHighDpiScreen);
216227
Clipper.logger.logEvent(regionSelectionProcessingEvent);
217228
});
218229
}
@@ -248,6 +259,8 @@ class RegionSelectorClass extends ComponentBase<RegionSelectorState, ClipperStat
248259
return Promise.reject(new Error("Expected the two points to be set, but they were not"));
249260
}
250261

262+
const devicePixelRatio = this.devicePixelRatio;
263+
251264
return new Promise<HTMLCanvasElement>((resolve) => {
252265
let regionSelectionLoadingEvent = new Log.Event.BaseEvent(Log.Event.Label.RegionSelectionLoading);
253266
let img: HTMLImageElement = new Image();
@@ -264,17 +277,16 @@ class RegionSelectorClass extends ComponentBase<RegionSelectorState, ClipperStat
264277
let destinationOffsetY = 0;
265278
let width = (xMax - xMin);
266279
let height = (yMax - yMin);
267-
let sourceOffsetX = xMin * this.devicePixelRatio;
268-
let sourceOffsetY = yMin * this.devicePixelRatio;
269-
let sourceWidth = (xMax - xMin) * this.devicePixelRatio;
270-
let sourceHeight = (yMax - yMin) * this.devicePixelRatio;
280+
let sourceOffsetX = xMin * devicePixelRatio;
281+
let sourceOffsetY = yMin * devicePixelRatio;
282+
let sourceWidth = (xMax - xMin) * devicePixelRatio;
283+
let sourceHeight = (yMax - yMin) * devicePixelRatio;
271284

272285
let canvas: HTMLCanvasElement = document.createElement("canvas") as HTMLCanvasElement;
273286
canvas.width = width;
274287
canvas.height = height;
275288
let ctx: CanvasRenderingContext2D = canvas.getContext("2d");
276289
ctx.drawImage(img, sourceOffsetX, sourceOffsetY, sourceWidth, sourceHeight, destinationOffsetX, destinationOffsetY, width, height);
277-
278290
resolve(canvas);
279291
};
280292

src/scripts/logging/submodules/propertyName.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export module PropertyName {
2828
InitialDataUrlLength,
2929
InvokeMode,
3030
InvokeSource,
31+
IsHighDpiScreen,
3132
IsRetryable,
3233
IsSerif,
3334
Key,

0 commit comments

Comments
 (0)