Clean up horizontal reveal sequence, do not touch the DOM in onScrollHeightChanged

src/vs/base/browser/styleMutator.ts

@@ -226,13 +226,17 @@ export const StyleMutator = {
 		let desiredValue = top + 'px';
 		if (domNode.style.top !== desiredValue) {
 			domNode.style.top = desiredValue;
+			return true;
 		}
+		return false;
 	},
 	setLeft: (domNode: HTMLElement, left: number) => {
 		let desiredValue = left + 'px';
 		if (domNode.style.left !== desiredValue) {
 			domNode.style.left = desiredValue;
+			return true;
 		}
+		return false;
 	},
 	setBottom: (domNode: HTMLElement, bottom: number) => {
 		let desiredValue = bottom + 'px';
@@ -272,17 +276,21 @@ export const StyleMutator = {
 };
 
 // Define setTransform
-function setWebkitTransform(domNode: HTMLElement, desiredValue: string): void {
+function setWebkitTransform(domNode: HTMLElement, desiredValue: string): boolean {
 	if (domNode.getAttribute('data-transform') !== desiredValue) {
 		domNode.setAttribute('data-transform', desiredValue);
 		(<any>domNode.style).webkitTransform = desiredValue;
+		return true;
 	}
+	return false;
 }
-function setTransform(domNode: HTMLElement, desiredValue: string): void {
+function setTransform(domNode: HTMLElement, desiredValue: string): boolean {
 	if (domNode.getAttribute('data-transform') !== desiredValue) {
 		domNode.setAttribute('data-transform', desiredValue);
 		domNode.style.transform = desiredValue;
+		return true;
 	}
+	return false;
 }
 (function() {
 	let testDomNode = document.createElement('div');

src/vs/editor/browser/view/viewImpl.ts

@@ -868,19 +868,24 @@ export class View extends ViewEventHandler implements editorBrowser.IView, IDisp
 		return r;
 	}
 
+	private _getViewPartsToRender(): ViewPart[] {
+		let result:ViewPart[] = [];
+		for (let i = 0, len = this.viewParts.length; i < len; i++) {
+			let viewPart = this.viewParts[i];
+			if (viewPart.shouldRender()) {
+				result.push(viewPart);
+			}
+		}
+		return result;
+	}
+
 	private _actualRender(): void {
 		if (!dom.isInDOM(this.domNode)) {
 			return;
 		}
 		let t = timer.start(timer.Topic.EDITOR, 'View.render');
 
-		let viewPartsToRender:ViewPart[] = [];
-		for (let i = 0, len = this.viewParts.length; i < len; i++) {
-			let viewPart = this.viewParts[i];
-			if (viewPart.shouldRender()) {
-				viewPartsToRender.push(viewPart);
-			}
-		}
+		let viewPartsToRender = this._getViewPartsToRender();
 
 		if (!this.viewLines.shouldRender() && viewPartsToRender.length === 0) {
 			// Nothing to render
@@ -893,6 +898,9 @@ export class View extends ViewEventHandler implements editorBrowser.IView, IDisp
 		if (this.viewLines.shouldRender()) {
 			this.viewLines.renderText(linesViewportData);
 			this.viewLines.onDidRender();
+
+			// Rendering of viewLines might cause scroll events to occur, so collect view parts to render again
+			viewPartsToRender = this._getViewPartsToRender();
 		}
 
 		let renderingContext = this.createRenderingContext(linesViewportData);

src/vs/editor/browser/view/viewOverlays.ts

@@ -219,13 +219,15 @@ export class MarginViewOverlays extends ViewOverlays {
 	private _glyphMarginLeft:number;
 	private _glyphMarginWidth:number;
 	private _scrollHeight:number;
+	private _contentLeft: number;
 
 	constructor(context:editorBrowser.IViewContext, layoutProvider:editorBrowser.ILayoutProvider) {
 		super(context, layoutProvider);
 
-		this._glyphMarginLeft = 0;
-		this._glyphMarginWidth = 0;
+		this._glyphMarginLeft = context.configuration.editor.layoutInfo.glyphMarginLeft;
+		this._glyphMarginWidth = context.configuration.editor.layoutInfo.glyphMarginWidth;
 		this._scrollHeight = layoutProvider.getScrollHeight();
+		this._contentLeft = context.configuration.editor.layoutInfo.contentLeft;
 
 		this.domNode.setClassName(editorBrowser.ClassNames.MARGIN_VIEW_OVERLAYS + ' monaco-editor-background');
 		this.domNode.setWidth(1);
@@ -251,10 +253,6 @@ export class MarginViewOverlays extends ViewOverlays {
 
 	public onScrollHeightChanged(scrollHeight:number): boolean {
 		this._scrollHeight = scrollHeight;
-		var glyphMargin = this._getGlyphMarginDomNode();
-		if (glyphMargin) {
-			StyleMutator.setHeight(glyphMargin, this._scrollHeight);
-		}
 		return super.onScrollHeightChanged(scrollHeight) || true;
 	}
 
@@ -262,15 +260,7 @@ export class MarginViewOverlays extends ViewOverlays {
 		this._glyphMarginLeft = layoutInfo.glyphMarginLeft;
 		this._glyphMarginWidth = layoutInfo.glyphMarginWidth;
 		this._scrollHeight = this._layoutProvider.getScrollHeight();
-
-		this.domNode.setWidth(layoutInfo.contentLeft);
-
-		var glyphMargin = this._getGlyphMarginDomNode();
-		if (glyphMargin) {
-			StyleMutator.setLeft(glyphMargin, layoutInfo.glyphMarginLeft);
-			StyleMutator.setWidth(glyphMargin, layoutInfo.glyphMarginWidth);
-		}
-
+		this._contentLeft = layoutInfo.contentLeft;
 		return super.onLayoutChanged(layoutInfo) || true;
 	}
 
@@ -284,5 +274,13 @@ export class MarginViewOverlays extends ViewOverlays {
 		}
 		var height = Math.min(this._layoutProvider.getTotalHeight(), 1000000);
 		this.domNode.setHeight(height);
+		this.domNode.setWidth(this._contentLeft);
+
+		var glyphMargin = this._getGlyphMarginDomNode();
+		if (glyphMargin) {
+			StyleMutator.setHeight(glyphMargin, this._scrollHeight);
+			StyleMutator.setLeft(glyphMargin, this._glyphMarginLeft);
+			StyleMutator.setWidth(glyphMargin, this._glyphMarginWidth);
+		}
 	}
 }
\ No newline at end of file

src/vs/editor/browser/viewParts/lines/viewLines.ts

@@ -140,6 +140,7 @@ export class ViewLines extends ViewLayer {
 	public onLayoutChanged(layoutInfo:editorCommon.IEditorLayoutInfo): boolean {
 		var shouldRender = super.onLayoutChanged(layoutInfo);
 		this._maxLineWidth = 0;
+		this._lastRenderedData.resetDomNodeClientRectLeft();
 		return shouldRender;
 	}
 
@@ -379,39 +380,50 @@ export class ViewLines extends ViewLayer {
 			throw new Error('I did not ask to render!');
 		}
 
+		// (1) render lines - ensures lines are in the DOM
 		super._renderLines(linesViewportData);
-		this.onDidRender();
-
 		this._lastRenderedData.setBigNumbersDelta(linesViewportData.bigNumbersDelta);
 		this._lastRenderedData.setCurrentVisibleRange(linesViewportData.visibleRange);
-		this._lastRenderedData.resetDomNodeClientRectLeft();
+		this.domNode.setWidth(this._layoutProvider.getScrollWidth());
+		this.domNode.setHeight(Math.min(this._layoutProvider.getTotalHeight(), 1000000));
 
+		// (2) compute horizontal scroll position:
+		//  - this must happen after the lines are in the DOM since it might need a line that rendered just now
+		//  - it might change `scrollWidth` and `scrollLeft`
 		if (this._lastCursorRevealRangeHorizontallyEvent) {
-			var newScrollLeft = this._computeScrollLeftToRevealRange(this._lastCursorRevealRangeHorizontallyEvent.range);
+			let revealHorizontalRange = this._lastCursorRevealRangeHorizontallyEvent.range;
 			this._lastCursorRevealRangeHorizontallyEvent = null;
 
+			// allow `visibleRangesForRange2` to work
+			this.onDidRender();
+
+			// compute new scroll position
+			var newScrollLeft = this._computeScrollLeftToRevealRange(revealHorizontalRange);
+
 			var isViewportWrapping = this._isViewportWrapping;
 			if (!isViewportWrapping) {
+				// ensure `scrollWidth` is large enough
 				this._ensureMaxLineWidth(newScrollLeft.maxHorizontalOffset);
 			}
 
+			// set `scrollLeft`
 			this._layoutProvider.setScrollLeft(newScrollLeft.scrollLeft);
 		}
 
-		// Update max line width (not so important, it is just so the horizontal scrollbar doesn't get too small)
-		this._asyncUpdateLineWidths.schedule();
-
+		let somethingChanged = false;
 		if (browser.canUseTranslate3d) {
 			var transform = 'translate3d(' + -this._layoutProvider.getScrollLeft() + 'px, ' + linesViewportData.visibleRangesDeltaTop + 'px, 0px)';
-			StyleMutator.setTransform(<HTMLElement>this.domNode.domNode.parentNode, transform); // TODO@Alex
+			somethingChanged = StyleMutator.setTransform(<HTMLElement>this.domNode.domNode.parentNode, transform) || somethingChanged; // TODO@Alex
 		} else {
-			StyleMutator.setTop(<HTMLElement>this.domNode.domNode.parentNode, linesViewportData.visibleRangesDeltaTop); // TODO@Alex
-			StyleMutator.setLeft(<HTMLElement>this.domNode.domNode.parentNode, -this._layoutProvider.getScrollLeft()); // TODO@Alex
+			somethingChanged = StyleMutator.setTop(<HTMLElement>this.domNode.domNode.parentNode, linesViewportData.visibleRangesDeltaTop) || somethingChanged; // TODO@Alex
+			somethingChanged = StyleMutator.setLeft(<HTMLElement>this.domNode.domNode.parentNode, -this._layoutProvider.getScrollLeft()) || somethingChanged; // TODO@Alex
+		}
+		if (somethingChanged) {
+			this._lastRenderedData.resetDomNodeClientRectLeft();
 		}
-		this._lastRenderedData.resetDomNodeClientRectLeft();
 
-		this.domNode.setWidth(this._layoutProvider.getScrollWidth());
-		this.domNode.setHeight(Math.min(this._layoutProvider.getTotalHeight(), 1000000));
+		// Update max line width (not so important, it is just so the horizontal scrollbar doesn't get too small)
+		this._asyncUpdateLineWidths.schedule();
 	}
 
 	// --- width