Commit d8eedc34 authored by Robert Knight's avatar Robert Knight

Remove iframe size check

Since we now only enable annotation of iframes that are explicitly opted
in by the containing page, we can avoid checking the size of the iframe
as well. That check was originally added to filter out ads and hidden
iframes.

This resolves an issue with Epub.js where an iframe initially has zero
width. This is because book content is laid out into a variable number
of fixed-width columns and the iframe's width is set to the total width
of all columns. Until the content is loaded, the number of columns and
hence the width, is zero.
parent 0c73c43e
...@@ -55,20 +55,14 @@ function shouldEnableAnnotation(iframe) { ...@@ -55,20 +55,14 @@ function shouldEnableAnnotation(iframe) {
// Ignore the Hypothesis sidebar. // Ignore the Hypothesis sidebar.
const isNotClientFrame = !iframe.classList.contains('h-sidebar-iframe'); const isNotClientFrame = !iframe.classList.contains('h-sidebar-iframe');
// Ignore hidden or very small iframes. // Require iframes to opt into annotation support.
const frameRect = iframe.getBoundingClientRect(); //
const MIN_WIDTH = 150; // Eventually we may want annotation to be enabled by default for iframes that
const MIN_HEIGHT = 150; // pass certain tests. However we need to resolve a number of issues before we
const hasSizableContainer =
frameRect.width > MIN_WIDTH && frameRect.height > MIN_HEIGHT;
// Ignore frames which have not opted into annotation support.
// Eventually we would like iframe annotation to be enabled by default,
// however we need to resolve a number of issues with iframe support before we
// can do that. See https://github.com/hypothesis/client/issues/530 // can do that. See https://github.com/hypothesis/client/issues/530
const enabled = iframe.hasAttribute('enable-annotation'); const enabled = iframe.hasAttribute('enable-annotation');
return isNotClientFrame && hasSizableContainer && enabled; return isNotClientFrame && enabled;
} }
function isDocumentReady(iframe, callback) { function isDocumentReady(iframe, callback) {
......
...@@ -40,16 +40,6 @@ describe('annotator.util.frame-util', function () { ...@@ -40,16 +40,6 @@ describe('annotator.util.frame-util', function () {
assert.deepEqual(foundFrames, [frame1, frame2], 'appended frames should be found'); assert.deepEqual(foundFrames, [frame1, frame2], 'appended frames should be found');
}); });
it('should not return small frames', function () {
// add frames that are small in both demensions
_addFrameToContainer({width: 140});
_addFrameToContainer({height: 140});
const foundFrames = frameUtil.findFrames(container);
assert.lengthOf(foundFrames, 0, 'frames with small demensions should not be found');
});
it('should not return frames that have not opted into annotation', () => { it('should not return frames that have not opted into annotation', () => {
const frame = _addFrameToContainer(); const frame = _addFrameToContainer();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment