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

WIP: beforeinstallprompt #506

Closed
wants to merge 25 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
4af0719
WIP: beforeinstallprompt
marcoscaceres Oct 13, 2016
58d8ba8
Add BIP JS implementation
marcoscaceres Oct 19, 2016
be3a60c
Using Matt's suggested var names
marcoscaceres Oct 19, 2016
6217a17
Make spec match ref implementation
marcoscaceres Oct 20, 2016
1e8b410
Add example
marcoscaceres Oct 20, 2016
366fd5b
Add example
marcoscaceres Oct 20, 2016
8626382
Dont await prompt
marcoscaceres Oct 20, 2016
7feafc2
move onbeforeinstallprompt to match IDL
marcoscaceres Oct 20, 2016
0e51b64
Add rejection of userChoice on error
marcoscaceres Oct 20, 2016
0ac6715
Add completely loaded from HTML5
marcoscaceres Oct 20, 2016
d25fadf
Markup fixes
marcoscaceres Oct 20, 2016
128edcf
Fix if/else error behavior
marcoscaceres Oct 20, 2016
4517699
Fixes based on discussion
marcoscaceres Oct 20, 2016
556e8de
Fixup implementations and tests based on mgiuca feedback
marcoscaceres Oct 20, 2016
0c5a224
fixes based on @mgiuca's feedback
marcoscaceres Oct 21, 2016
9663d86
Fixup based on feedback
marcoscaceres Oct 23, 2016
ea98a82
Linking and cleanup
marcoscaceres Oct 23, 2016
7802c95
Fixed heading, as suggeted by @kenchris
marcoscaceres Oct 23, 2016
b6e5043
cross ref construct a BeforeInstallPromptEvent
marcoscaceres Oct 23, 2016
38a2bab
MUST instead of SHOULD
marcoscaceres Oct 23, 2016
561182b
MUST instead of SHOULD
marcoscaceres Oct 23, 2016
5f65182
Fixed typo
marcoscaceres Oct 23, 2016
7e10680
Fixup based on recent comments
marcoscaceres Oct 24, 2016
8f89507
fixes(bip implementation): based on feedback
marcoscaceres Oct 24, 2016
2f86eb6
nits from @kenchris
marcoscaceres Oct 24, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
162 changes: 162 additions & 0 deletions implementation/BeforeInstallPromptEvent.js
@@ -0,0 +1,162 @@
/*globals DOMException*/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this file be called *Polyfill.js or similar?

"use strict"; {
const internalSlots = new WeakMap();
const installProcesses = [];
const AppBannerPromptOutcome = new Set([
"accepted",
"dismissed",
]);

/**
* Emulates the UI thread firing the event.
* @param {BeforeInstallPromptEvent} event
* @param {Element} element A DOM element that simulates the pop-up.
* @return {Promise<void>} Resolves when user makes a choice.
*/
async function requestInstallPrompt(event, element) {
const internal = internalSlots.get(event);
internal.didPrompt = true;
const promptOutcome = await showInstallPrompt(element);
internal.userChoiceResolver(promptOutcome);
}

function hasValidUserChoice({ userChoice }) {
if (typeof userChoice === "undefined") {
return true;
}
if (!AppBannerPromptOutcome.has(String(userChoice))) {
return false;
}
return true;
}

/**
* Implementation of BeforeInstallPromptEvent.
*
*/
class BeforeInstallPromptEvent extends Event {
constructor(typeArg, eventInit) {
// WebIDL Guard. Not in spec, as it's all handled by WebIDL.
if (arguments.length === 0) {
throw new TypeError("Not enough arguments. Expected at least 1.");
}
const initType = typeof eventInit;
if (arguments.length === 2 && initType !== "undefined" && initType !== "object") {
throw new TypeError("Value can't be converted to a dictionary.");
}
super(typeArg, Object.assign({ cancelable: true }, eventInit));

if (eventInit && !hasValidUserChoice(eventInit)) {
const msg = `The provided value '${eventInit.userChoice}' is not a valid` +
Copy link
Collaborator

Choose a reason for hiding this comment

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

` works for multiline

" enum value of type AppBannerPromptOutcome.";
throw new TypeError(msg);
}
// End WebIDL guard.
const internal = {
didPrompt: false,
userChoice: null,
userChoiceResolver: null, // Implicit in spec
};

internal.userChoice = new Promise(resolve => {
if (eventInit && "userChoice" in eventInit) {
return resolve(eventInit.userChoice);
}
internal.userChoiceResolver = resolve;
});
internalSlots.set(this, internal);
}

prompt() {
if (this.isTrusted === false) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This still requires a real, trusted BIP event. I'm not quite sure what you're trying to do here because this can never truly work (i.e., this is inside the definition of the polyfill BeforeInstallPromptEvent yet its own prompt method requires a real BeforeInstallPromptEvent).

Are you trying to make this a sort of formal JavaScript language version of the spec ("how it should be") even though it can't work? Or are you trying to make a polyfill that can actually be used? You could do the simulatedIsTrusted thing I suggested on the previous review round to make this actually work.

Your response to my suggestion was:

Within a few weeks, we will have both Chrome and Firefox implementing this for real in C++, so let's keep the current sample implementation as is.

But that doesn't change the fact that this sample implementation is non-functional.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, it's as functional as any other user-constructed event that relies on trust. You are correct that it's crippled - but that's again by design: you can still construct these events and fire them, but it's just "prompt()" that doesn't do anything - but that will be exactly the same for the real ones when constructed by developers. In that sense, what is currently implemented is correct per-spec.

Note that I could, theoretically, take BeforeInstallPromptEvent.js and plug it into Gecko, because we support JS-based IDL bindings. So, in such an implementation I would get a "trusted" event.

const msg = "Untrusted events can't call prompt().";
throw new DOMException(msg, "NotAllowedError");
}

if (this.defaultPrevented === false) {
const msg = ".prompt() needs to be called after .preventDefault()";
throw new DOMException(msg, "InvalidStateError");
}

if (internalSlots.get(this).didPrompt) {
const msg = ".prompt() can only be successfully called once.";
throw new DOMException(msg, "InvalidStateError");
}
requestInstallPrompt(this);
}

get userChoice() {
return internalSlots.get(this).userChoice;
}
}

async function notifyBeforeInstallPrompt(element) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about calling this simulateBeforeInstallPrompt?

(Since it is not part of the BIP API surface, it's just used by the buttons on the test page to simulate an action the UA normally takes.)

Copy link
Member Author

Choose a reason for hiding this comment

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

This is supposed to match "the steps to notify before an automated install" - which is why I named it as such (admittedly, not the best name). I'll add a @see link to the code.

if (document.readyState !== "complete") {
await waitUntilReadyStateComplete();
}
if (installProcesses.length) {
return;
}
const event = new BeforeInstallPromptEvent("beforeinstallprompt");
window.dispatchEvent(event);
if (!event.defaultPrevented) {
await requestInstallPrompt(event, element);
}
}

function waitUntilReadyStateComplete() {
return new Promise(resolve => {
document.addEventListener("readystatechange", () => {
if (document.readyState === "complete") {
resolve();
}
});
});
}

async function showInstallPrompt(button) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

the functon calling this called the argument element, why not just call it button there as well

if (button) {
button.disabled = true;
}
await Promise.all(installProcesses);
const prompt = document.createElement("fieldset");
prompt.id = "installprompt";
prompt.innerHTML = `
<h2>Add to Home screen</h2>
<p>... the foo app ...</p>
<button id="cancel">CANCEL</button>
<button id="add">ADD</button>
`;
const cancel = prompt.querySelector("#cancel");
const add = prompt.querySelector("#add");
document.body.appendChild(prompt);
const p = new Promise((resolve) => {
add.addEventListener("click", () => {
resolve("accepted");
// Emulate installation to home screen
setTimeout(() => {
window.dispatchEvent(new Event("appinstalled"));
}, 1000);
});
cancel.addEventListener("click", () => resolve("dismissed"));
Copy link
Collaborator

Choose a reason for hiding this comment

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

if it has default arguments and you don't care abut them, then I prefer _ over ()

});
cancel.onclick = add.onclick = function() {
document.body.removeChild(prompt);
installProcesses.splice(installProcesses.findIndex(item => item === p), 1);
if (button) {
button.disabled = false;
}
};
installProcesses.push(p);
return p;
}

if (!window.BeforeInstallPromptEvent) {
window.BeforeInstallPromptEvent = BeforeInstallPromptEvent;
} else {
console.warn("Using browser's implementation of BeforeInstallPromptEvent.");
}

window.notifyBeforeInstallPrompt = notifyBeforeInstallPrompt;
window.showInstallPrompt = showInstallPrompt;
}
214 changes: 214 additions & 0 deletions implementation/index.html
@@ -0,0 +1,214 @@
<!doctype html>
<meta charset="utf-8">
<script src="BeforeInstallPromptEvent.js">
</script>
<style>
@keyframes fadein {
from {
opacity: 0;
}
to {
opacity: 1;
}
}

#installprompt {
animation: fadein 2s;
width: 10cm;
padding: 1cm;
}
</style>
<button onclick="notifyBeforeInstallPrompt(this)">
Simulate install BeforeInstallPrompt
</button>
<button onclick="showInstallPrompt(this)">
Simulate manual "Add to Home Screen"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe add a bit of explanation to what that means

</button>
<script>
/*globals BeforeInstallPromptEvent, DOMException*/
"use strict";
const e = new BeforeInstallPromptEvent("");
try {
new BeforeInstallPromptEvent();
console.assert(false, "Must throw when constructed with no arguments");
} catch (err) {}

try {
const typeArgs = [
"",
"\t\n",
"beforeinstallprompt",
12,
null,
undefined,
];
for (const testArg of typeArgs) {
const {
type
} = new BeforeInstallPromptEvent(testArg);
const expected = String(testArg);
const result = type === expected;
console.assert(result, `type attribute is '${type}', expected '${expected}'`);
}
} catch (err) {
console.assert(false, "Unexpected exception when constructing BeforeInstallPromptEvent", err);
}

try {
new BeforeInstallPromptEvent("");
new BeforeInstallPromptEvent("", undefined);
new BeforeInstallPromptEvent("", null);
} catch (err) {
console.assert(false, "empty optional eventInit options must not throw", err);
}

try {
new BeforeInstallPromptEvent("", "");
console.assert(false, "Must throw when eventInit is the empty string");
} catch (err) {}

try {
new BeforeInstallPromptEvent("", 123);
console.assert(false, "Must throw when eventInit is number");
} catch (err) {}

try {
new BeforeInstallPromptEvent("", {
userChoice: undefined
});
new BeforeInstallPromptEvent("", {
userChoice: "accepted"
});
new BeforeInstallPromptEvent("", {
userChoice: "dismissed"
});
} catch (err) {
console.assert(false, "threw on valid AppBannerPromptOutcome enum values", err);
}

const b1 = new BeforeInstallPromptEvent("", {
userChoice: "accepted"
});
const b2 = new BeforeInstallPromptEvent("", {
userChoice: "dismissed"
});
Promise
.all([b1.userChoice, b2.userChoice])
.then(([accepted, dismissed]) => {
const isAccepted = accepted === "accepted";
console.assert(isAccepted, "Expected 'accepted', got '${accepted}'");
const isDismissed = dismissed === "dismissed";
console.assert(isDismissed, "Expected 'dismissed', got '${dismissed}'");
})
.catch(err => console.assert(false, "Unexpected promise rejection", err));

const invalidUserChoiceValues = [
null,
123,
"not-a-valid-option"
];
for (const invalidValue of invalidUserChoiceValues) {
try {
new BeforeInstallPromptEvent("", {
userChoice: invalidValue
});
console.assert(false, `Must throw on invalid userChoice value: ${invalidValue}`);
} catch (err) {}
}

console.assert(e.cancelable === true, "cancelable attribute must be true by default");
const e2 = new BeforeInstallPromptEvent("", {
cancelable: true
});
console.assert(e2.cancelable === true, "Must reflect cancelable dict. option of true");
const e3 = new BeforeInstallPromptEvent("", {
cancelable: false
});
console.assert(e3.cancelable === false, "Must reflect cancelable dict. option of false");

const hasAttr = "userChoice" in e;
console.assert(hasAttr, "BeforeInstallPromptEvent must have a userChoice attribute");

const isPromise = e.userChoice instanceof Promise;
console.assert(isPromise, "userChoice must be a Promise");

console.assert("prompt" in e, "BeforeInstallPromptEvent must have a prompt() method");
console.assert(typeof e.prompt === "function", "prompt() must be a function");
console.assert(e.prompt.length === 0, "prompt() must take no arguments");


try {
e.prompt();
console.assert(false, "calling prompt() on an untrusted event must throw");
} catch (err) {
console.assert(err instanceof DOMException, "Exception must be a DOMException");
const isNotAllowedErr = err.name === "NotAllowedError";
console.assert(isNotAllowedErr, `Caught '${err.name}', expected 'NotAllowedError'`);
}

window.addEventListener("beforeinstallprompt", test1, {
once: true
});

function test1(e) {
console.info("Running test 1");
try {
e.prompt();
console.assert(false, "Must throw when preventDefault() not called first");
} catch (err) {
const isInvalidState = err.name === "InvalidStateError";
console.assert(isInvalidState, `Caught '${err.name}', expected 'InvalidStateError'`);
}
try {
e.preventDefault();
e.prompt();
window.addEventListener("appinstalled", setupTest2, {
once: true
});
} catch (err) {
console.assert(false, "Unexpected exceptions");
}
try {
e.prompt();
console.assert(false, "Calling prompt() twice must throw");
} catch (err) {
const isInvalidState = err.name === "InvalidStateError";
console.assert(isInvalidState, `Caught '${err.name}', expected 'InvalidStateError'`);
}
}

function test2(ev) {
console.log("Running test 2");
// run tests
const isComplete = document.readyState === "complete";
console.assert(isComplete, "Must only fire after document is fully loaded");

const isBIP = ev instanceof BeforeInstallPromptEvent;
console.assert(isBIP, "Must be instance of BeforeInstallPromptEvent");
console.assert(ev.isTrusted, "Must be a trusted event");

ev.preventDefault();
const isUndefined = typeof ev.prompt() === "undefined";
console.assert(isUndefined, "Calling prompt() must return undefined");
try {
ev.prompt();
console.assert(false, "calling prompt() more than once must throw");
} catch (err) {
console.assert(err instanceof DOMException, "expected a DOMException");
const isInvalidState = err.name === "InvalidStateError";
console.assert(isInvalidState, `Caught '${err.name}', expected 'InvalidStateError'`);
}
}

function setupTest2() {
console.log("setting up test 2");
window.addEventListener("beforeinstallprompt", test2, {
once: true
});
}

// install events
window.addEventListener("appinstalled", () => {
console.info("Application installed successfully");
});
</script>