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

XFA - Keep xfa layer on top of the others (bug 1719629) #13706

Merged
merged 1 commit into from
Jul 9, 2021

Conversation

calixteman
Copy link
Contributor

No description provided.

@calixteman calixteman requested a review from Snuffleupagus July 9, 2021 16:18
@calixteman calixteman merged commit 90e5279 into mozilla:master Jul 9, 2021
@calixteman calixteman deleted the 13697 branch July 9, 2021 18:10
Comment on lines +499 to +502
if (this.xfaLayer?.div) {
// The xfa layer needs to stay on top.
div.appendChild(this.xfaLayer.div);
}
Copy link
Collaborator

@Snuffleupagus Snuffleupagus Jul 9, 2021

Choose a reason for hiding this comment

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

If I'd actually been given a chance to review this before it landed, I'd have suggested to move this block and place it around line 513 instead rather than here since that'd seem generally "safer"!

With this patch, you're not actually guaranteed a correct placement (i.e. on top) of the XFALayer in the DOM in the event that the textLayer is disabled. This probably won't matter in practice, since I'm hoping that XFA documents don't contain a separate AnnotationLayer (although how knows with XFA Foreground documents).

(Since as was asked for review this, and the patch was opened a mere two hours ago, I guess that I don't really understand the big rush here!?)

Copy link
Contributor

Choose a reason for hiding this comment

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

(Since as was asked for review this, and the patch was opened a mere two hours ago, I guess that I don't really understand the big rush here!?)

We're trying to land as many XFA fixes as possible as we hope to enable XFA in Firefox 91.

calixteman added a commit to calixteman/pdf.js that referenced this pull request Jul 9, 2021
calixteman added a commit that referenced this pull request Jul 9, 2021
XFA - Move xfa layer on top of the others (follow-up of #13706)
bh213 pushed a commit to bh213/pdf.js that referenced this pull request Jun 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants