Skip to content
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

Set the text fields font size based on their height #14936

Merged
merged 1 commit into from
May 28, 2022
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
10 changes: 5 additions & 5 deletions src/core/annotation.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
escapeString,
getModificationDate,
isAscii,
LINE_FACTOR,
OPS,
RenderingIntentFlag,
shadow,
Expand Down Expand Up @@ -55,11 +56,6 @@ import { StringStream } from "./stream.js";
import { writeDict } from "./writer.js";
import { XFAFactory } from "./xfa/factory.js";

// Represent the percentage of the height of a single-line field over
// the font size.
// Acrobat seems to use this value.
const LINE_FACTOR = 1.35;

class AnnotationFactory {
/**
* Create an `Annotation` object of the correct type for the given reference
Expand Down Expand Up @@ -1921,6 +1917,7 @@ class TextWidgetAnnotation extends WidgetAnnotation {
!this.hasFieldFlag(AnnotationFieldFlag.PASSWORD) &&
!this.hasFieldFlag(AnnotationFieldFlag.FILESELECT) &&
this.data.maxLen !== null;
this.data.doNotScroll = this.hasFieldFlag(AnnotationFieldFlag.DONOTSCROLL);
}

_getCombAppearance(defaultAppearance, font, text, width, hPadding, vPadding) {
Expand Down Expand Up @@ -2788,6 +2785,9 @@ class LinkAnnotation extends Annotation {
this.data.quadPoints = quadPoints;
}

// The color entry for a link annotation is the color of the border.
this.data.borderColor = this.data.borderColor || this.data.color;

Catalog.parseDestDictionary({
destDict: params.dict,
resultObj: this.data,
Expand Down
103 changes: 67 additions & 36 deletions src/display/annotation_layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
AnnotationBorderStyleType,
AnnotationType,
assert,
LINE_FACTOR,
shadow,
unreachable,
Util,
Expand All @@ -37,6 +38,7 @@ import { ColorConverters } from "../shared/scripting_utils.js";
import { XfaLayer } from "./xfa_layer.js";

const DEFAULT_TAB_INDEX = 1000;
const DEFAULT_FONT_SIZE = 9;
const GetElementsByNameSet = new WeakSet();

function getRectDims(rect) {
Expand Down Expand Up @@ -271,12 +273,12 @@ class AnnotationElement {
break;
}

const borderColor = data.borderColor || data.color || null;
const borderColor = data.borderColor || null;
if (borderColor) {
container.style.borderColor = Util.makeHexColor(
data.color[0] | 0,
data.color[1] | 0,
data.color[2] | 0
borderColor[0] | 0,
borderColor[1] | 0,
borderColor[2] | 0
);
} else {
// Transparent (invisible) border, so do not draw it at all.
Expand Down Expand Up @@ -897,6 +899,53 @@ class WidgetAnnotationElement extends AnnotationElement {
? "transparent"
: Util.makeHexColor(color[0], color[1], color[2]);
}

/**
* Apply text styles to the text in the element.
*
* @private
* @param {HTMLDivElement} element
* @memberof TextWidgetAnnotationElement
*/
_setTextStyle(element) {
const TEXT_ALIGNMENT = ["left", "center", "right"];
const { fontColor } = this.data.defaultAppearanceData;
const fontSize =
this.data.defaultAppearanceData.fontSize || DEFAULT_FONT_SIZE;

const style = element.style;

// TODO: If the font-size is zero, calculate it based on the height and
// width of the element.
// Not setting `style.fontSize` will use the default font-size for now.

// We don't use the font, as specified in the PDF document, for the <input>
// element. Hence using the original `fontSize` could look bad, which is why
// it's instead based on the field height.
// If the height is "big" then it could lead to a too big font size
// so in this case use the one we've in the pdf (hence the min).
if (this.data.multiLine) {
const height = Math.abs(this.data.rect[3] - this.data.rect[1]);
const numberOfLines = Math.round(height / (LINE_FACTOR * fontSize)) || 1;
const lineHeight = height / numberOfLines;
style.fontSize = `${Math.min(
fontSize,
Math.round(lineHeight / LINE_FACTOR)
)}px`;
} else {
const height = Math.abs(this.data.rect[3] - this.data.rect[1]);
style.fontSize = `${Math.min(
fontSize,
Math.round(height / LINE_FACTOR)
)}px`;
}

style.color = Util.makeHexColor(fontColor[0], fontColor[1], fontColor[2]);

if (this.data.textAlignment !== null) {
style.textAlign = TEXT_ALIGNMENT[this.data.textAlignment];
}
}
}

class TextWidgetAnnotationElement extends WidgetAnnotationElement {
Expand Down Expand Up @@ -944,10 +993,16 @@ class TextWidgetAnnotationElement extends WidgetAnnotationElement {
if (this.data.multiLine) {
element = document.createElement("textarea");
element.textContent = textContent;
if (this.data.doNotScroll) {
element.style.overflowY = "hidden";
}
} else {
element = document.createElement("input");
element.type = "text";
element.setAttribute("value", textContent);
if (this.data.doNotScroll) {
element.style.overflowX = "hidden";
}
}
GetElementsByNameSet.add(element);
element.disabled = this.data.readOnly;
Expand Down Expand Up @@ -1177,32 +1232,6 @@ class TextWidgetAnnotationElement extends WidgetAnnotationElement {
this.container.appendChild(element);
return this.container;
}

/**
* Apply text styles to the text in the element.
*
* @private
* @param {HTMLDivElement} element
* @memberof TextWidgetAnnotationElement
*/
_setTextStyle(element) {
const TEXT_ALIGNMENT = ["left", "center", "right"];
const { fontSize, fontColor } = this.data.defaultAppearanceData;
const style = element.style;

// TODO: If the font-size is zero, calculate it based on the height and
// width of the element.
// Not setting `style.fontSize` will use the default font-size for now.
if (fontSize) {
style.fontSize = `${fontSize}px`;
}

style.color = Util.makeHexColor(fontColor[0], fontColor[1], fontColor[2]);

if (this.data.textAlignment !== null) {
style.textAlign = TEXT_ALIGNMENT[this.data.textAlignment];
}
}
}

class CheckboxWidgetAnnotationElement extends WidgetAnnotationElement {
Expand Down Expand Up @@ -1413,10 +1442,8 @@ class ChoiceWidgetAnnotationElement extends WidgetAnnotationElement {
value: this.data.fieldValue,
});

let { fontSize } = this.data.defaultAppearanceData;
if (!fontSize) {
fontSize = 9;
}
const fontSize =
this.data.defaultAppearanceData.fontSize || DEFAULT_FONT_SIZE;
const fontSizeStyle = `calc(${fontSize}px * var(--zoom-factor))`;

const selectElement = document.createElement("select");
Expand All @@ -1426,8 +1453,6 @@ class ChoiceWidgetAnnotationElement extends WidgetAnnotationElement {
selectElement.setAttribute("id", id);
selectElement.tabIndex = DEFAULT_TAB_INDEX;

selectElement.style.fontSize = `${fontSize}px`;

if (!this.data.combo) {
// List boxes have a size and (optionally) multiple selection.
selectElement.size = this.data.options.length;
Expand Down Expand Up @@ -1606,6 +1631,12 @@ class ChoiceWidgetAnnotationElement extends WidgetAnnotationElement {
});
}

if (this.data.combo) {
this._setTextStyle(selectElement);
} else {
// Just use the default font size...
// it's a bit hard to guess what is a good size.
}
this._setBackgroundColor(selectElement);
this._setDefaultPropertiesFromJS(selectElement);

Expand Down
5 changes: 5 additions & 0 deletions src/shared/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ import "./compatibility.js";
const IDENTITY_MATRIX = [1, 0, 0, 1, 0, 0];
const FONT_IDENTITY_MATRIX = [0.001, 0, 0, 0.001, 0, 0];

// Represent the percentage of the height of a single-line field over
// the font size. Acrobat seems to use this value.
const LINE_FACTOR = 1.35;

/**
* Refer to the `WorkerTransport.getRenderingIntent`-method in the API, to see
* how these flags are being used:
Expand Down Expand Up @@ -1162,6 +1166,7 @@ export {
isArrayBuffer,
isArrayEqual,
isAscii,
LINE_FACTOR,
MissingPDFException,
objectFromMap,
objectSize,
Expand Down
4 changes: 3 additions & 1 deletion test/driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,9 @@ class Driver {
transform,
};
if (renderForms) {
renderContext.annotationMode = AnnotationMode.ENABLE_FORMS;
renderContext.annotationMode = task.annotationStorage
? AnnotationMode.ENABLE_STORAGE
: AnnotationMode.ENABLE_FORMS;
} else if (renderPrint) {
if (task.annotationStorage) {
renderContext.annotationMode = AnnotationMode.ENABLE_STORAGE;
Expand Down
2 changes: 2 additions & 0 deletions test/pdfs/issue14301.pdf.link
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
https://github.com/mozilla/pdf.js/files/7594316/formulairecerfa90nov00-1.pdf

21 changes: 21 additions & 0 deletions test/test_manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -6507,5 +6507,26 @@
"rounds": 1,
"link": true,
"type": "other"
},
{ "id": "issue14301",
"file": "pdfs/issue14301.pdf",
"md5": "9973936dcf8dd41daa62c04a4eb621f0",
"rounds": 1,
"link": true,
"firstPage": 2,
"lastPage": 2,
"type": "eq",
"forms": true,
"annotationStorage": {
"1832R": {
"value": "3"
},
"1827R": {
"value": "2"
},
"1808R": {
"value": "1"
}
}
}
]
6 changes: 1 addition & 5 deletions web/annotation_layer_builder.css
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,9 @@
background-image: var(--annotation-unfocused-field-background);
border: 1px solid transparent;
box-sizing: border-box;
font-size: 9px;
font: 9px sans-serif;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this essentially "sneaking" in a fix for issue #14736 here?

If so, that's probably good to mention explicitly in the commit message (and linking the issue such that it's closed).

height: 100%;
margin: 0;
padding: 0 3px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this essentially "sneaking" in a fix for issue #14301 here?

If so, that's probably good to mention explicitly in the commit message (and linking the issue such that it's closed).


Given the discussion in the issue, does this generally work without problems?
Is this properly covered by existing tests, or does new reference test(s) need to be added?

vertical-align: top;
width: 100%;
}
Expand All @@ -76,8 +75,6 @@
}

.annotationLayer .textWidgetAnnotation textarea {
font: message-box;
font-size: 9px;
resize: none;
}

Expand Down Expand Up @@ -167,7 +164,6 @@
.annotationLayer .buttonWidgetAnnotation.checkBox input,
.annotationLayer .buttonWidgetAnnotation.radioButton input {
appearance: none;
padding: 0;
}

.annotationLayer .popupWrapper {
Expand Down