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.

Sign up for GitHub

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

extension: Fix some minor issues #12917

Merged
merged 2 commits into from
Aug 26, 2023
Merged

extension: Fix some minor issues #12917

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
359e638
extension: Fix HTML structure of 'open in new tab' message
n0samu Aug 23, 2023
2e73876
extension: Don't show CORS error on internal player page
n0samu Aug 23, 2023
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
19 changes: 13 additions & 6 deletions web/packages/core/src/ruffle-player.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1994,24 +1994,31 @@ export class RufflePlayer extends HTMLElement {
});
}
this.hideSplashScreen();

const div = document.createElement("div");
div.id = "message_overlay";
const innerDiv = document.createElement("div");
innerDiv.className = "message";
innerDiv.innerHTML = textAsParagraphs("message-cant-embed");
Copy link
Contributor

@danielhjacobs danielhjacobs Aug 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This can be innerText or textContent now, right? Or is message-cant-embed an HTML element? We should be moving away from innerHTML.

Copy link
Contributor

@danielhjacobs danielhjacobs Aug 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Oh no, I see textAsParagraphs returns an HTML string. It should return an HTMLDivElement though ideally, so you can just appendChild the result.

Copy link
Contributor

Choose a reason for hiding this comment

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

IIUC this should be fixed in #12937, right? Let's merge this as-is for now.


const buttonDiv = document.createElement("div");
const link = document.createElement("a");
link.innerText = text("open-in-new-tab");
link.onclick = () => openInNewTab(url);
buttonDiv.appendChild(link);

div.innerHTML = `<div class="message">
${textAsParagraphs("message-cant-embed")}
<div></div>
</div>`;
div.lastChild!.appendChild(link);
innerDiv.appendChild(buttonDiv);
div.appendChild(innerDiv);
this.container.prepend(div);
} else {
const error = new Error("Failed to fetch: " + this.swfUrl);
if (!this.swfUrl!.protocol.includes("http")) {
error.ruffleIndexError = PanicError.FileProtocol;
} else if (window.location.origin === this.swfUrl!.origin) {
} else if (
window.location.origin === this.swfUrl!.origin ||
// The extension's internal player page is not restricted by CORS
window.location.protocol.includes("extension")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this part work for Edge?

Copy link
Member Author

@n0samu n0samu Aug 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, Edge's extension protocol is called extension:. I've never seen a Chromium-based browser that used an extension protocol not including the word "extension" in its name.

EDIT: Oh that's funny, Edge actually uses chrome-extension: internally but displays it in the address bar as extension:.

) {
error.ruffleIndexError = PanicError.SwfFetchError;
} else {
// This is a selfhosted build of Ruffle that tried to make a cross-origin request
Expand Down