From 9c81e87e841aaa17a95837f3c7a98b752282a6db Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sun, 1 Nov 2015 19:23:08 +0100 Subject: [PATCH 1/3] :bug: Fix incorrectly reported width when measuring lines This commit fixes what seems to be a bug in Chromium. When measuring lines (and with a special character sequence), it could happen that Range(0, 0). getBoundingClientRect().width reports a number greater than 0. This seems to happen when the font size is smaller than 12px and it's probably due to subpixel font scaling. To solve it we've explicitly included a guard clause that prevents this problem to happen. --- spec/lines-yardstick-spec.coffee | 35 ++++++++++++++++++++++++++++---- src/lines-yardstick.coffee | 9 +++++--- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/spec/lines-yardstick-spec.coffee b/spec/lines-yardstick-spec.coffee index 022f535f4..ae85a0e9d 100644 --- a/spec/lines-yardstick-spec.coffee +++ b/spec/lines-yardstick-spec.coffee @@ -2,7 +2,7 @@ LinesYardstick = require "../src/lines-yardstick" {toArray} = require 'underscore-plus' describe "LinesYardstick", -> - [editor, mockPresenter, mockLineNodesProvider, createdLineNodes, linesYardstick] = [] + [editor, mockPresenter, mockLineNodesProvider, createdLineNodes, linesYardstick, buildLineNode] = [] beforeEach -> waitsForPromise -> @@ -49,10 +49,10 @@ describe "LinesYardstick", -> buildLineNode(screenRow) textNodesForLineIdAndScreenRow: (lineId, screenRow) -> lineNode = @lineNodeForLineIdAndScreenRow(lineId, screenRow) + iterator = document.createNodeIterator(lineNode, NodeFilter.SHOW_TEXT) textNodes = [] - for span in lineNode.children - for textNode in span.childNodes - textNodes.push(textNode) + while textNode = iterator.nextNode() + textNodes.push(textNode) textNodes editor.setLineHeightInPixels(14) @@ -126,6 +126,33 @@ describe "LinesYardstick", -> expect(linesYardstick.pixelPositionForScreenPosition([0, 9]).left).toBe 67 expect(linesYardstick.pixelPositionForScreenPosition([0, 11]).left).toBe 84 + it "doesn't report a width greater than 0 when the character to measure is at the beginning of a text node", -> + # This spec documents what seems to be a bug in Chromium, because we'd + # expect that Range(0, 0).getBoundingClientRect().width to always be zero. + atom.styles.addStyleSheet """ + * { + font-size: 11px; + font-family: monospace; + } + """ + + text = " \\vec{w}_j^r(\\text{new}) &= \\vec{w}_j^r(\\text{old}) + \\Delta\\vec{w}_j^r, \\\\" + buildLineNode = (screenRow) -> + lineNode = document.createElement("div") + lineNode.style.whiteSpace = "pre" + # We couldn't reproduce the problem with a simple string, so we're + # attaching the full one that comes from a bug report. + lineNode.innerHTML = ' \\vec{w}_j^r(\\text{new}) &= \\vec{w}_j^r(\\text{old}) + \\Delta\\vec{w}_j^r, \\\\' + jasmine.attachToDOM(lineNode) + createdLineNodes.push(lineNode) + lineNode + + editor.setText(text) + + expect(linesYardstick.pixelPositionForScreenPosition([0, 35]).left).toBe 230.90625 + expect(linesYardstick.pixelPositionForScreenPosition([0, 36]).left).toBe 237.5 + expect(linesYardstick.pixelPositionForScreenPosition([0, 37]).left).toBe 244.09375 + it "doesn't measure invisible lines if it is explicitly told so", -> atom.styles.addStyleSheet """ * { diff --git a/src/lines-yardstick.coffee b/src/lines-yardstick.coffee index fa00bae40..54ba6cf57 100644 --- a/src/lines-yardstick.coffee +++ b/src/lines-yardstick.coffee @@ -159,9 +159,12 @@ class LinesYardstick 0 leftPixelPositionForCharInTextNode: (lineNode, textNode, charIndex) -> - @rangeForMeasurement.setStart(textNode, 0) - @rangeForMeasurement.setEnd(textNode, charIndex) - width = @rangeForMeasurement.getBoundingClientRect().width + if charIndex is 0 + width = 0 + else + @rangeForMeasurement.setStart(textNode, 0) + @rangeForMeasurement.setEnd(textNode, charIndex) + width = @rangeForMeasurement.getBoundingClientRect().width @rangeForMeasurement.setStart(textNode, 0) @rangeForMeasurement.setEnd(textNode, textNode.textContent.length) From 8f44bb1d4f9a73afd8a18edbb664394fab9d2bee Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 2 Nov 2015 18:42:04 +0100 Subject: [PATCH 2/3] :arrow_up: tabs --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 6be01f438..5c0d1b8c1 100644 --- a/package.json +++ b/package.json @@ -108,7 +108,7 @@ "status-bar": "0.80.0", "styleguide": "0.45.0", "symbols-view": "0.110.0", - "tabs": "0.87.0", + "tabs": "0.88.0", "timecop": "0.33.0", "tree-view": "0.195.0", "update-package-dependencies": "0.10.0", From 7aa913f4aece3f41298aa65ad01b7145a5cd93c1 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 2 Nov 2015 18:48:30 +0100 Subject: [PATCH 3/3] 1.2.0-beta1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 5c0d1b8c1..9e01f5b3a 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "atom", "productName": "Atom", - "version": "1.2.0-beta0", + "version": "1.2.0-beta1", "description": "A hackable text editor for the 21st Century.", "main": "./src/browser/main.js", "repository": {