Skip to content

Commit

Permalink
Simplify the updatetextlayermatches event handling in `TextLayerBui…
Browse files Browse the repository at this point in the history
…lder`

This implements the nice suggestion from mozilla#10099 (comment), which I at the time didn't think would work.
I'll probably have to plead temporary insanity here, since it *should* have been totally obvious to me how this could be implemented. By simply not registering the event until the `textLayer` is actually rendered, removing the event on `cancel` works just fine.

This patch also removes the `pagecancelled` event, given that it's no longer used anywhere in the code-base and that its implemention was flawed since it wasn't guaranteed to be called in *every* situation where rendering was actually cancelled.
  • Loading branch information
Snuffleupagus authored and lpillonel committed Feb 22, 2019
1 parent 3b959ab commit d2e1747
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 47 deletions.
10 changes: 0 additions & 10 deletions web/pdf_page_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,6 @@ class PDFPageView {
}

cancelRendering(keepAnnotations = false) {
const renderingState = this.renderingState;

if (this.paintTask) {
this.paintTask.cancel();
this.paintTask = null;
Expand All @@ -276,14 +274,6 @@ class PDFPageView {
this.annotationLayer.cancel();
this.annotationLayer = null;
}

if (renderingState !== RenderingStates.INITIAL) {
this.eventBus.dispatch('pagecancelled', {
source: this,
pageNumber: this.id,
renderingState,
});
}
}

cssTransform(target, redrawAnnotations = false) {
Expand Down
53 changes: 16 additions & 37 deletions web/text_layer_builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,7 @@ class TextLayerBuilder {
this.textLayerRenderTask = null;
this.enhanceTextSelection = enhanceTextSelection;

this._boundEvents = Object.create(null);
this._bindEvents();

this._onUpdateTextLayerMatches = null;
this._bindMouse();
}

Expand Down Expand Up @@ -109,6 +107,16 @@ class TextLayerBuilder {
}, function (reason) {
// Cancelled or failed to render text layer; skipping errors.
});

if (!this._onUpdateTextLayerMatches) {
this._onUpdateTextLayerMatches = (evt) => {
if (evt.pageIndex === this.pageIdx || evt.pageIndex === -1) {
this._updateMatches();
}
};
this.eventBus.on('updatetextlayermatches',
this._onUpdateTextLayerMatches);
}
}

/**
Expand All @@ -119,6 +127,11 @@ class TextLayerBuilder {
this.textLayerRenderTask.cancel();
this.textLayerRenderTask = null;
}
if (this._onUpdateTextLayerMatches) {
this.eventBus.off('updatetextlayermatches',
this._onUpdateTextLayerMatches);
this._onUpdateTextLayerMatches = null;
}
}

setTextContentStream(readableStream) {
Expand Down Expand Up @@ -314,40 +327,6 @@ class TextLayerBuilder {
this._renderMatches(this.matches);
}

/**
* @private
*/
_bindEvents() {
const { eventBus, _boundEvents, } = this;

_boundEvents.pageCancelled = (evt) => {
if (evt.pageNumber !== this.pageNumber) {
return;
}
if (this.textLayerRenderTask) {
console.error('TextLayerBuilder._bindEvents: `this.cancel()` should ' +
'have been called when the page was reset, or rendering cancelled.');
return;
}
// Ensure that all event listeners are cleaned up when the page is reset,
// since re-rendering will create new `TextLayerBuilder` instances and the
// number of (stale) event listeners would otherwise grow without bound.
for (const name in _boundEvents) {
eventBus.off(name.toLowerCase(), _boundEvents[name]);
delete _boundEvents[name];
}
};
_boundEvents.updateTextLayerMatches = (evt) => {
if (evt.pageIndex !== this.pageIdx && evt.pageIndex !== -1) {
return;
}
this._updateMatches();
};

eventBus.on('pagecancelled', _boundEvents.pageCancelled);
eventBus.on('updatetextlayermatches', _boundEvents.updateTextLayerMatches);
}

/**
* Improves text selection by adding an additional div where the mouse was
* clicked. This reduces flickering of the content if the mouse is slowly
Expand Down

0 comments on commit d2e1747

Please sign in to comment.