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

Don't steal the copy event (bug 1857823) #17099

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
113 changes: 113 additions & 0 deletions test/integration/copy_paste_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ describe("Copy and paste", () => {
);
});
});

describe("Copy/paste and ligatures", () => {
let pages;

Expand Down Expand Up @@ -197,4 +198,116 @@ describe("Copy and paste", () => {
);
});
});

describe("Copy/paste and ligatures (but without selecting all)", () => {
let pages;

beforeAll(async () => {
pages = await loadAndWait(
"copy_paste_ligatures.pdf",
"#hiddenCopyElement",
100,
async page => {
await page.waitForFunction(
() => !!window.PDFViewerApplication.eventBus
);
await waitForTextLayer(page);
}
);
});

afterAll(async () => {
await closePages(pages);
});

it("must check that the ligatures have been removed when the text has been copied", async () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We already have a test-case in this file with the same exact name, which we've seen recently can be annoying when trying to track down intermittent issues; so please change this one slightly.

await Promise.all(
pages.map(async ([browserName, page]) => {
await page.evaluate(async () => {
const promise = new Promise(resolve => {
document.addEventListener("selectionchange", resolve, {
once: true,
});
});
const range = document.createRange();
range.selectNodeContents(document.querySelector(".textLayer"));
const selection = window.getSelection();
selection.addRange(range);
await promise;
});
const promise = page.evaluate(
() =>
new Promise(resolve => {
document.addEventListener(
"copy",
e => resolve(e.clipboardData.getData("text/plain")),
{
once: true,
}
);
})
);
await page.keyboard.down("Control");
await page.keyboard.press("c");
await page.keyboard.up("Control");
const text = await promise;

if (browserName === "chrome") {
// In Firefox the copy event hasn't the correct value.
Comment on lines +255 to +256
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you please add a TODO with a link to a relevant upstream bug-report, either in Firefox or Puppeteer (whichever is appropriate), so that we can eventually enable this test everywhere?

expect(text)
.withContext(`In ${browserName}`)
.toEqual("abcdeffffiflffifflſtstghijklmno");
}
})
);
});
});

describe("Copy event", () => {
let pages;

beforeAll(async () => {
pages = await loadAndWait(
"tracemonkey.pdf",
".textLayer .endOfContent",
100,
async page => {
await page.waitForFunction(
() => !!window.PDFViewerApplication.eventBus
);
await waitForTextLayer(page);
}
);
});

afterAll(async () => {
await closePages(pages);
});

it("must check that the copy event is not stolen (bug 1857823)", async () => {
await Promise.all(
pages.map(async ([browserName, page]) => {
await page.evaluate(async () => {
const promise = new Promise(resolve => {
document.addEventListener("selectionchange", resolve, {
once: true,
});
});
const range = document.createRange();
const span = document.querySelector(".textLayer span");
range.selectNode(span);
const selection = window.getSelection();
selection.addRange(range);
await promise;
});

const promise = waitForEvent(page, "copy", 300000);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is ten times larger than the Jasmine-timeout, is that intentional?

await page.keyboard.down("Control");
await page.keyboard.press("c");
await page.keyboard.up("Control");
await promise;
})
);
});
});
});
31 changes: 20 additions & 11 deletions web/text_layer_builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -218,17 +218,26 @@ class TextLayerBuilder {
end.classList.remove("active");
});

div.addEventListener("copy", event => {
if (!this.#enablePermissions) {
const selection = document.getSelection();
event.clipboardData.setData(
"text/plain",
removeNullCharacters(normalizeUnicode(selection.toString()))
);
}
event.preventDefault();
event.stopPropagation();
});
div.addEventListener(
"copy",
event => {
if (!this.#enablePermissions) {
const selection = document.getSelection();
event.clipboardData.setData(
"text/plain",
removeNullCharacters(normalizeUnicode(selection.toString()))
);
} else {
event.stopPropagation();
}
// It's required after the clipboard data have been changed else the
// clipboard will contain the selection!
event.preventDefault();
},
// Capture the event in order to be sure that the copy event has the
// correct value.
{ capture: true }
);
}
}

Expand Down