jenkins-bot has submitted this change and it was merged.
Change subject: Make CSS transplantation in OO.ui.Frame actually wait for CSS
......................................................................
Make CSS transplantation in OO.ui.Frame actually wait for CSS
Before e9ca44c86, there was code in place that waited for the CSS
to load and fired 'initialize' asynchronously. It was initially removed
because it was deemed unnecessary, but with later changes it has become
necessary again. The original approach also didn't work in Chrome.
This commit completely rewrites the CSS transplantation code to
correctly wait for CSS to be loaded before emitting 'initialize',
and to unbreak image URL references by no longer inlining same-origin
styles.
The new code transplants <style> tags directly, while <link> tags
are rewritten as <style>@import url(..); #foo { ... }</style>. We then
insert a <div id="foo"> into the document and wait for it to get styled,
which will only happen after the @import has finished. For the styling
of #foo we use font-family, because it allows us to set arbitrary
string values.
Additionally, we have to use visiblity: hidden; to hide windows
initially, because display: none; causes the iframe to not load in
Firefox. We reapply display: none; after the iframe initializes.
Bug: 56630
Change-Id: I76688a5ad7bd144c5e46064119f607060e76f0d9
---
M src/OO.ui.Frame.js
M src/OO.ui.Window.js
M src/styles/OO.ui.Window.css
3 files changed, 104 insertions(+), 61 deletions(-)
Approvals:
Krinkle: Looks good to me, approved
jenkins-bot: Verified
diff --git a/src/OO.ui.Frame.js b/src/OO.ui.Frame.js
index 3c6fb32..6a761fe 100644
--- a/src/OO.ui.Frame.js
+++ b/src/OO.ui.Frame.js
@@ -42,6 +42,76 @@
* @event initialize
*/
+/* Static Methods */
+
+/**
+ * Transplant the CSS styles from as parent document to a frame's document.
+ *
+ * This loops over the style sheets in the parent document, and copies their
nodes to the
+ * frame's document. It then polls the document to see when all styles have
loaded, and once they
+ * have, invokes the callback.
+ *
+ * For details of how we arrived at the strategy used in this function, see
#load.
+ *
+ * @static
+ * @method
+ * @inheritable
+ * @param {HTMLDocument} parentDoc Document to transplant styles from
+ * @param {HTMLDocument} frameDoc Document to transplant styles to
+ * @param {Function} [callback] Callback to execute once styles have loaded
+ */
+OO.ui.Frame.static.transplantStyles = function ( parentDoc, frameDoc, callback
) {
+ var i, numSheets, styleNode, newNode, timeout, pollNodeId,
$pendingPollNodes,
+ $pollNodes = $( [] ),
+ // Fake font-family value
+ fontFamily = 'oo-ui-frame-transplantStyles-loaded';
+
+ for ( i = 0, numSheets = parentDoc.styleSheets.length; i < numSheets;
i++ ) {
+ styleNode = parentDoc.styleSheets[i].ownerNode;
+ if ( callback && styleNode.nodeName.toLowerCase() === 'link' ) {
+ // External stylesheet
+ // Create a node with a unique ID that we're going to
monitor to see when the CSS
+ // has loaded
+ pollNodeId = 'oo-ui-frame-transplantStyles-loaded-' + i;
+ $pollNodes = $pollNodes.add( $( '<div>', frameDoc )
+ .attr( 'id', pollNodeId )
+ .appendTo( frameDoc.body )
+ );
+
+ // Add <style>@import url(...); #pollNodeId {
font-family: ... }</style>
+ // The font-family rule will only take effect once the
@import finishes
+ newNode = frameDoc.createElement( 'style' );
+ newNode.textContent = '@import url(' + styleNode.href +
');\n' +
+ '#' + pollNodeId + ' { font-family: ' +
fontFamily + '; }';
+ } else {
+ // Not an external stylesheet, or no polling required;
just copy the node over
+ newNode = frameDoc.importNode( styleNode, true );
+ }
+ frameDoc.head.appendChild( newNode );
+ }
+
+ if ( callback ) {
+ // Poll every 100ms until all external stylesheets have loaded
+ $pendingPollNodes = $pollNodes;
+ timeout = setTimeout( function pollExternalStylesheets() {
+ while (
+ $pendingPollNodes.length > 0 &&
+ $pendingPollNodes.eq( 0 ).css( 'font-family' )
=== fontFamily
+ ) {
+ $pendingPollNodes = $pendingPollNodes.slice( 1
);
+ }
+
+ if ( $pendingPollNodes.length === 0 ) {
+ // We're done!
+ $pollNodes.remove();
+ callback();
+ } else {
+ timeout = setTimeout( pollExternalStylesheets,
100 );
+ }
+ }, 100 );
+ }
+};
+
/* Methods */
/**
@@ -55,20 +125,32 @@
* iframe is triggered when you call close, and there's no further load event
to indicate that
* everything is actually loaded.
*
- * By dynamically adding stylesheet links, we can detect when each link is
loaded by testing if we
- * have access to each of their `sheet.cssRules` properties. Every 10ms we
poll to see if we have
- * access to the style's `sheet.cssRules` property yet.
+ * In Chrome, stylesheets don't show up in document.styleSheets until they
have loaded, so we could
+ * just poll that array and wait for it to have the right length. However, in
Firefox, stylesheets
+ * are added to document.styleSheets immediately, and the only way you can
determine whether they've
+ * loaded is to attempt to access .cssRules and wait for that to stop throwing
an exception. But
+ * cross-domain stylesheets never allow .cssRules to be accessed even after
they have loaded.
*
- * However, because of security issues, we never have such access if the
stylesheet came from a
- * different site. Thus, we are left with linking to the stylesheets through a
style element with
- * multiple `@import` statements - which ends up being simpler anyway. Since
we created that style,
- * we always have access, and its contents are only available when everything
is done loading.
+ * The workaround is to change all `<link href="...">` tags to `<style>@import
url(...)</style>` tags.
+ * Because `@import` is blocking, Chrome won't add the stylesheet to
document.styleSheets until
+ * the `@import` has finished, and Firefox won't allow .cssRules to be
accessed until the `@import`
+ * has finished. And because the contents of the `<style>` tag are from the
same origin, accessing
+ * .cssRules is allowed.
+ *
+ * However, now that we control the styles we're injecting, we might as well
do away with
+ * browser-specific polling hacks like document.styleSheets and .cssRules, and
instead inject
+ * `<style>@import url(...); #foo { font-family: someValue; }</style>`, then
create `<div id="foo">`
+ * and wait for its font-family to change to someValue. Because `@import` is
blocking, the font-family
+ * rule is not applied until after the `@import` finishes.
+ *
+ * All this stylesheet injection and polling magic is in #transplantStyles.
*
* @fires initialize
*/
OO.ui.Frame.prototype.load = function () {
var win = this.$element.prop( 'contentWindow' ),
- doc = win.document;
+ doc = win.document,
+ frame = this;
// Figure out directionality:
this.dir = this.$element.closest( '[dir]' ).prop( 'dir' ) || 'ltr';
@@ -90,54 +172,12 @@
this.$content = this.$( '.oo-ui-frame-content' );
this.$document = this.$( doc );
- this.transplantStyles();
- this.initialized = true;
- this.emit( 'initialize' );
-};
-
-/**
- * Transplant the CSS styles from the frame's parent document to the frame's
document.
- *
- * This loops over the style sheets in the parent document, and copies their
tags to the
- * frame's document. `<link>` tags pointing to same-origin style sheets are
inlined as `<style>` tags;
- * `<link>` tags pointing to foreign URLs and `<style>` tags are copied
verbatim.
- */
-OO.ui.Frame.prototype.transplantStyles = function () {
- var i, ilen, j, jlen, sheet, rules, cssText, styleNode,
- newDoc = this.$document[0],
- parentDoc = this.getElementDocument();
- for ( i = 0, ilen = parentDoc.styleSheets.length; i < ilen; i++ ) {
- sheet = parentDoc.styleSheets[i];
- styleNode = undefined;
- try {
- rules = sheet.cssRules;
- } catch ( e ) { }
- if ( sheet.ownerNode.nodeName.toLowerCase() === 'link' && rules
) {
- // This is a <link> tag pointing to a same-origin style
sheet. Rebuild it as a
- // <style> tag. This needs to be in a try-catch because
it sometimes fails in Firefox.
- try {
- cssText = '';
- for ( j = 0, jlen = rules.length; j < jlen; j++
) {
- if ( typeof rules[j].cssText !==
'string' ) {
- // WTF; abort and fall back to
cloning the node
- throw new Error(
'sheet.cssRules[' + j + '].cssText is not a string' );
- }
- cssText += rules[j].cssText + '\n';
- }
- cssText += '/* Transplanted styles from ' +
sheet.href + ' */\n';
- styleNode = newDoc.createElement( 'style' );
- styleNode.textContent = cssText;
- } catch ( e ) {
- styleNode = undefined;
- }
+ this.constructor.static.transplantStyles( this.getElementDocument(),
this.$document[0],
+ function () {
+ frame.initialized = true;
+ frame.emit( 'initialize' );
}
- if ( !styleNode ) {
- // It's either a <style> tag or a <link> tag pointing
to a foreign URL; just copy
- // it to the new document
- styleNode = newDoc.importNode( sheet.ownerNode, true );
- }
- newDoc.body.appendChild( styleNode );
- }
+ );
};
/**
diff --git a/src/OO.ui.Window.js b/src/OO.ui.Window.js
index 39826d2..b87a65d 100644
--- a/src/OO.ui.Window.js
+++ b/src/OO.ui.Window.js
@@ -32,6 +32,9 @@
// Initialization
this.$element
.addClass( 'oo-ui-window' )
+ // Hide the window using visibility: hidden; while the iframe
is still loading
+ // Can't use display: none; because that prevents the iframe
from loading in Firefox
+ .css( 'visibility', 'hidden' )
.append( this.$frame );
this.$frame
.addClass( 'oo-ui-window-frame' )
@@ -268,6 +271,10 @@
this.$overlay
);
+ // Undo the visibility: hidden; hack from the constructor and apply
display: none;
+ // We can do this safely now that the iframe has initialized
+ this.$element.hide().css( 'visibility', '' );
+
this.emit( 'initialize' );
return this;
@@ -318,9 +325,9 @@
OO.ui.Window.prototype.open = function ( data ) {
if ( !this.opening && !this.closing && !this.visible ) {
this.opening = true;
- this.$element.show();
- this.visible = true;
this.frame.run( OO.ui.bind( function () {
+ this.$element.show();
+ this.visible = true;
this.frame.$element.focus();
this.emit( 'opening', data );
this.setup( data );
diff --git a/src/styles/OO.ui.Window.css b/src/styles/OO.ui.Window.css
index 77c7e94..0f9dc17 100644
--- a/src/styles/OO.ui.Window.css
+++ b/src/styles/OO.ui.Window.css
@@ -1,7 +1,3 @@
-.oo-ui-window {
- display: none;
-}
-
.oo-ui-window-head {
-webkit-touch-callout: none;
-webkit-user-select: none;
--
To view, visit https://gerrit.wikimedia.org/r/96431
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I76688a5ad7bd144c5e46064119f607060e76f0d9
Gerrit-PatchSet: 6
Gerrit-Project: oojs/ui
Gerrit-Branch: master
Gerrit-Owner: Catrope <roan.katt...@gmail.com>
Gerrit-Reviewer: Krinkle <krinklem...@gmail.com>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
