From f8b0e5592ba91ad9066fb0842794cf4696242147 Mon Sep 17 00:00:00 2001 From: Thomas Buckley-Houston Date: Tue, 18 Jun 2019 12:31:45 +0300 Subject: [PATCH] Text rendering: fixes and improvements Introduces a new option to disable the pixel comparison text visibility detecting code. There are some situations in which it helps and some in which it hinders. So for now it is disable by default and can be enabled through the config file or my pressing F6 in the TTY client. Also includes a couple of fixes to the HTTP server's text rendering. --- interfacer/src/browsh/config_sample.go | 5 +++++ webext/src/background/dimensions.js | 4 ++-- webext/src/dom/commands_mixin.js | 9 +++++++++ webext/src/dom/text_builder.js | 19 ++++++++++++++----- webext/src/dom/tty_grid.js | 6 +++++- 5 files changed, 35 insertions(+), 8 deletions(-) diff --git a/interfacer/src/browsh/config_sample.go b/interfacer/src/browsh/config_sample.go index 01202d8c..7c605534 100644 --- a/interfacer/src/browsh/config_sample.go +++ b/interfacer/src/browsh/config_sample.go @@ -17,6 +17,11 @@ mobile_user_agent = "Mozilla/5.0 (Android 7.0; Mobile; rv:54.0) Gecko/58.0 Firef [browsh] # Browsh internals websocket-port = 3334 +# Possibly better handling of overlapping text in web pages. If a page seems to have +# text that shouldn't be visible, if it should be behind another element for example, +# then this experimental feature should help. It can also be toggled in-browser with F6. +use_experimental_text_visibility = false + [firefox] # The path to your Firefox binary path = "firefox" diff --git a/webext/src/background/dimensions.js b/webext/src/background/dimensions.js index 21c2b229..0f80c33d 100644 --- a/webext/src/background/dimensions.js +++ b/webext/src/background/dimensions.js @@ -28,11 +28,11 @@ export default class extends utils.mixins(CommonMixin) { this.char.width != incoming.width || this.char.height != incoming.height ) { - this.char = _.clone(incoming); this.log( `Requesting browser resize for new char dimensions: ` + - `${this.char.width}x${this.char.height}` + `${incoming.width}x${incoming.height} (old: ${this.char.width}x${this.char.height})` ); + this.char = _.clone(incoming); this.resizeBrowserWindow(); } } diff --git a/webext/src/dom/commands_mixin.js b/webext/src/dom/commands_mixin.js index f037c57a..505c44b8 100644 --- a/webext/src/dom/commands_mixin.js +++ b/webext/src/dom/commands_mixin.js @@ -76,10 +76,19 @@ export default MixinBase => } _handleSpecialKeys(input) { + let state, message; switch (input.key) { case 18: // CTRL+r window.location.reload(); break; + case 284: // F6 + state = this.config.browsh.use_experimental_text_visibility; + state = !state; + this.config.browsh.use_experimental_text_visibility = state; + message = state ? 'on' : 'off' + this.sendMessage(`/status,info,Experimental text visibility: ${message}`); + this.sendSmallTextFrame(); + break; } } diff --git a/webext/src/dom/text_builder.js b/webext/src/dom/text_builder.js index 48973ce8..12806973 100644 --- a/webext/src/dom/text_builder.js +++ b/webext/src/dom/text_builder.js @@ -15,7 +15,7 @@ export default class extends utils.mixins(CommonMixin, SerialiseMixin) { this.dimensions = dimensions; this.graphics_builder = graphics_builder; this.config = config; - this.tty_grid = new TTYGrid(dimensions, graphics_builder); + this.tty_grid = new TTYGrid(dimensions, graphics_builder, config); this._parse_started_elements = []; // A `range` is the DOM's representation of elements and nodes as they are rendered in // the DOM. Think of the 'range' that is created when you select/highlight text for @@ -35,8 +35,8 @@ export default class extends utils.mixins(CommonMixin, SerialiseMixin) { buildFormattedText(callback) { this._updateState(); - this._getTextNodes(); this.graphics_builder.getOnOffScreenshots(() => { + this._getTextNodes(); this._positionTextNodes(); callback(); }); @@ -162,7 +162,9 @@ export default class extends utils.mixins(CommonMixin, SerialiseMixin) { // * Yet another thing, the style change doesn't actually get picked up until the // next frame. Thus why the loop is independent of the `positionTextNodes()` loop. _fixJustifiedText() { - this._node.parentElement.style.textAlign = "left"; + if (this._node.parentElement) { + this._node.parentElement.style.textAlign = "left"; + } } // The need for this wasn't immediately obvious to me. The fact is that the DOM stores @@ -246,8 +248,15 @@ export default class extends utils.mixins(CommonMixin, SerialiseMixin) { // Although do note that, unlike selection ranges, sub-selections can appear seemingly // inside other selections for things like italics or anchor tags. _getNodeDOMBoxes() { - this._range.selectNode(this._node); - return this._range.getClientRects(); + let rects = [] + // TODO: selectNode() hangs if it can't find a node in the DOM + // Node.isConnected() might be faster + // It's possible that the node has dissapeared since nodes were collected. + if (document.body.contains(this._node)){ + this._range.selectNode(this._node); + rects = this._range.getClientRects(); + } + return rects; } // A single box is always a valid rectangle. Therefore a single box will, for example, diff --git a/webext/src/dom/tty_grid.js b/webext/src/dom/tty_grid.js index aaba6e3b..39bdecf1 100644 --- a/webext/src/dom/tty_grid.js +++ b/webext/src/dom/tty_grid.js @@ -2,9 +2,10 @@ import utils from "utils"; // The TTY grid export default class { - constructor(dimensions, graphics_builder) { + constructor(dimensions, graphics_builder, config) { this.dimensions = dimensions; this.graphics_builder = graphics_builder; + this.config = config; this._setMiddleOfEm(); } @@ -98,6 +99,9 @@ export default class { // case we can work with `z-index` so that characters justifiably overwrite each other in // the TTY grid. _isCharObscured(colours) { + if (!this.config.browsh.use_experimental_text_visibility) { + return false + } return ( colours[0][0] === colours[1][0] && colours[0][1] === colours[1][1] &&