Skip to content

Let non-viewable Popup Annotations inherit the parent's Annotation Flags if the parent is viewable #7352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 25, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 43 additions & 19 deletions src/core/annotation.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,28 +190,49 @@ var Annotation = (function AnnotationClosure() {
}

Annotation.prototype = {
/**
* @private
*/
_hasFlag: function Annotation_hasFlag(flags, flag) {
return !!(flags & flag);
},

/**
* @private
*/
_isViewable: function Annotation_isViewable(flags) {
return !this._hasFlag(flags, AnnotationFlag.INVISIBLE) &&
!this._hasFlag(flags, AnnotationFlag.HIDDEN) &&
!this._hasFlag(flags, AnnotationFlag.NOVIEW);
},

/**
* @private
*/
_isPrintable: function AnnotationFlag_isPrintable(flags) {
return this._hasFlag(flags, AnnotationFlag.PRINT) &&
!this._hasFlag(flags, AnnotationFlag.INVISIBLE) &&
!this._hasFlag(flags, AnnotationFlag.HIDDEN);
},

/**
* @return {boolean}
*/
get viewable() {
if (this.flags) {
return !this.hasFlag(AnnotationFlag.INVISIBLE) &&
!this.hasFlag(AnnotationFlag.HIDDEN) &&
!this.hasFlag(AnnotationFlag.NOVIEW);
if (this.flags === 0) {
return true;
}
return true;
return this._isViewable(this.flags);
},

/**
* @return {boolean}
*/
get printable() {
if (this.flags) {
return this.hasFlag(AnnotationFlag.PRINT) &&
!this.hasFlag(AnnotationFlag.INVISIBLE) &&
!this.hasFlag(AnnotationFlag.HIDDEN);
if (this.flags === 0) {
return false;
}
return false;
return this._isPrintable(this.flags);
},

/**
Expand All @@ -224,11 +245,7 @@ var Annotation = (function AnnotationClosure() {
* @see {@link shared/util.js}
*/
setFlags: function Annotation_setFlags(flags) {
if (isInt(flags)) {
this.flags = flags;
} else {
this.flags = 0;
}
this.flags = (isInt(flags) && flags > 0) ? flags : 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this also address #7280, at least partially?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose so, together with the slight change of the _hasFlag method in the new patch.
I should perhaps not have sneaked this into the current patch, but given that I needed to touch most of those methods anyway, it seemed better to reduce code churn a bit.

},

/**
Expand All @@ -242,10 +259,7 @@ var Annotation = (function AnnotationClosure() {
* @see {@link shared/util.js}
*/
hasFlag: function Annotation_hasFlag(flag) {
if (this.flags) {
return (this.flags & flag) > 0;
}
return false;
return this._hasFlag(this.flags, flag);
},

/**
Expand Down Expand Up @@ -823,6 +837,16 @@ var PopupAnnotation = (function PopupAnnotationClosure() {
this.setColor(parentItem.getArray('C'));
this.data.color = this.color;
}

// If the Popup annotation is not viewable, but the parent annotation is,
// that is most likely a bug. Fallback to inherit the flags from the parent
// annotation (this is consistent with the behaviour in Adobe Reader).
if (!this.viewable) {
var parentFlags = parentItem.get('F');
if (this._isViewable(parentFlags)) {
this.setFlags(parentFlags);
}
}
}

Util.inherit(PopupAnnotation, Annotation, {});
Expand Down
1 change: 1 addition & 0 deletions test/pdfs/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@
!pr4922.pdf
!pr6531_1.pdf
!pr6531_2.pdf
!pr7352.pdf
!bug900822.pdf
!issue918.pdf
!issue1905.pdf
Expand Down
Binary file added test/pdfs/pr7352.pdf
Binary file not shown.
8 changes: 8 additions & 0 deletions test/test_manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -1838,6 +1838,14 @@
"type": "eq",
"annotations": true
},
{ "id": "pr7352",
"file": "pdfs/pr7352.pdf",
"md5": "336abca4b313cb215b0569883f1f683d",
"link": false,
"rounds": 1,
"type": "eq",
"annotations": true
},
{ "id": "issue1002",
"file": "pdfs/issue1002.pdf",
"md5": "af62d6cd95079322d4af18edd960d15c",
Expand Down
50 changes: 39 additions & 11 deletions test/unit/annotation_layer_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,17 @@ describe('Annotation layer', function() {
}
};

var annotationFactory;

beforeAll(function (done) {
annotationFactory = new AnnotationFactory();
done();
});

afterAll(function () {
annotationFactory = null;
});

describe('Annotation', function() {
it('should set and get flags', function() {
var dict = new Dict();
Expand Down Expand Up @@ -185,17 +196,6 @@ describe('Annotation layer', function() {
});

describe('LinkAnnotation', function() {
var annotationFactory;

beforeAll(function (done) {
annotationFactory = new AnnotationFactory();
done();
});

afterAll(function () {
annotationFactory = null;
});

it('should correctly parse a URI action', function() {
var actionDict = new Dict();
actionDict.set('Type', Name.get('Action'));
Expand Down Expand Up @@ -358,4 +358,32 @@ describe('Annotation layer', function() {
expect(annotation.file.content).toEqual(stringToBytes('Test attachment'));
});
});

describe('PopupAnnotation', function() {
it('should inherit the parent flags when the Popup is not viewable, ' +
'but the parent is (PR 7352)', function () {
var parentDict = new Dict();
parentDict.set('Type', Name.get('Annot'));
parentDict.set('Subtype', Name.get('Text'));
parentDict.set('F', 28); // viewable

var popupDict = new Dict();
popupDict.set('Type', Name.get('Annot'));
popupDict.set('Subtype', Name.get('Popup'));
popupDict.set('F', 25); // not viewable
popupDict.set('Parent', parentDict);

var xrefMock = new XrefMock([popupDict]);
var popupRef = new Ref(13, 0);

var popupAnnotation = annotationFactory.create(xrefMock, popupRef);
var data = popupAnnotation.data;
expect(data.annotationType).toEqual(AnnotationType.POPUP);

// Should not modify the `annotationFlags` returned e.g. through the API.
expect(data.annotationFlags).toEqual(25);
// The Popup should inherit the `viewable` property of the parent.
expect(popupAnnotation.viewable).toEqual(true);
});
});
});