chrome/browser/resources/security_warnings/interstitial_v2.html - Issue 830743003: Directly inline shared WebUI CSS declarations into the security and supervised user interstitials.

Side by Side Diff: chrome/browser/resources/security_warnings/interstitial_v2.html

Issue 830743003: Directly inline shared WebUI CSS declarations into the security and supervised user interstitials. (Closed) Base URL: https://chromium.googlesource.com/chromium/src@master

Patch Set: Created 5 years, 11 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

« no previous file with comments | « chrome/browser/interstitials/security_interstitial_page.cc ('k') | chrome/browser/resources/supervised_user_block_interstitial.html » ('j') | chrome/browser/resources/supervised_user_block_interstitial.html » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
1 <!doctype html>	1 <!doctype html>

2 <html i18n-values="dir:textdirection;">	2 <html>
jungshik at Google 2015/01/10 00:54:04 You have to keep this because 'dir' attribute in H You have to keep this because 'dir' attribute in HTML plays a different role than 'text-direction:' in CSS. In most cases, the former works better. Anyway, our CSS has a lot of selectors based on HTML's 'dir' ( body['dir=rtl']) so that we should keep 'dir. Bernhard Bauer 2015/01/12 15:47:17 Oh, ok. That does mean a few things though: 1. W Show quoted text On 2015/01/10 00:54:04, Jungshik at google wrote: > You have to keep this because 'dir' attribute in HTML plays a different role > than 'text-direction:' in CSS. In most cases, the former works better. > > Anyway, our CSS has a lot of selectors based on HTML's 'dir' ( body['dir=rtl']) > so that we should keep 'dir. Oh, ok. That does mean a few things though: 1. We should remove the fontFamily and fontSize keys from the dictionary returned by SetFontAndTextDirection(), and then rename it to SetTextDirection(). There seem to be some cases left though where those keys are used, so we'll need to fix those first. 2. What is the direction: attribute in the CSS file then used for? Shouldn't we automatically set that attribute via a [dir=rtl] selector somewhere in a shared CSS file? 3. For RTL text, the layout will still be somewhat prone to initial flicker, right? Is there a plan to get rid of that as well? Bernhard Bauer 2015/01/14 18:14:23 Jungshik, ping? Show quoted text On 2015/01/12 15:47:17, Bernhard Bauer wrote: > On 2015/01/10 00:54:04, Jungshik at google wrote: > > You have to keep this because 'dir' attribute in HTML plays a different role > > than 'text-direction:' in CSS. In most cases, the former works better. > > > > Anyway, our CSS has a lot of selectors based on HTML's 'dir' ( > body['dir=rtl']) > > so that we should keep 'dir. > > Oh, ok. That does mean a few things though: > > 1. We should remove the fontFamily and fontSize keys from the dictionary > returned by SetFontAndTextDirection(), and then rename it to SetTextDirection(). > There seem to be some cases left though where those keys are used, so we'll need > to fix those first. > 2. What is the direction: attribute in the CSS file then used for? Shouldn't we > automatically set that attribute via a [dir=rtl] selector somewhere in a shared > CSS file? > 3. For RTL text, the layout will still be somewhat prone to initial flicker, > right? Is there a plan to get rid of that as well? Jungshik, ping? Dan Beam 2015/01/21 19:21:41 this pattern is common: default styles { ma Show quoted text On 2015/01/14 18:14:23, Bernhard Bauer wrote: > On 2015/01/12 15:47:17, Bernhard Bauer wrote: > > On 2015/01/10 00:54:04, Jungshik at google wrote: > > > You have to keep this because 'dir' attribute in HTML plays a different role > > > than 'text-direction:' in CSS. In most cases, the former works better. > > > > > > Anyway, our CSS has a lot of selectors based on HTML's 'dir' ( > > body['dir=rtl']) > > > so that we should keep 'dir. > > > > Oh, ok. That does mean a few things though: > > > > 1. We should remove the fontFamily and fontSize keys from the dictionary > > returned by SetFontAndTextDirection(), and then rename it to > SetTextDirection(). > > There seem to be some cases left though where those keys are used, so we'll > need > > to fix those first. > > 2. What is the direction: attribute in the CSS file then used for? Shouldn't > we > > automatically set that attribute via a [dir=rtl] selector somewhere in a > shared > > CSS file? this pattern is common: default styles { make a box anchored to the left } if rtl { anchor it to the right by flipping some mutually exclusive styles } [dir='rtl'] is our current way to do "if rtl", which keys off an explicit HTML attribute being set. I don't know a way this can be accomplished via CSS only (as I don't know if CSS has the ability to derive the computed direction, just key off of stuff set by JS or HTML, e.g. classes and attributes). Show quoted text > > 3. For RTL text, the layout will still be somewhat prone to initial flicker, > > right? Is there a plan to get rid of that as well? I'm unsure of how to fix flicker with our current approach other than pre-processing. Show quoted text > > Jungshik, ping? Bernhard Bauer 2015/01/21 22:47:50 We could still do: [dir='rtl'] { direction: Show quoted text On 2015/01/21 19:21:41, Dan Beam wrote: > On 2015/01/14 18:14:23, Bernhard Bauer wrote: > > On 2015/01/12 15:47:17, Bernhard Bauer wrote: > > > On 2015/01/10 00:54:04, Jungshik at google wrote: > > > > You have to keep this because 'dir' attribute in HTML plays a different > role > > > > than 'text-direction:' in CSS. In most cases, the former works better. > > > > > > > > Anyway, our CSS has a lot of selectors based on HTML's 'dir' ( > > > body['dir=rtl']) > > > > so that we should keep 'dir. > > > > > > Oh, ok. That does mean a few things though: > > > > > > 1. We should remove the fontFamily and fontSize keys from the dictionary > > > returned by SetFontAndTextDirection(), and then rename it to > > SetTextDirection(). > > > There seem to be some cases left though where those keys are used, so we'll > > need > > > to fix those first. > > > 2. What is the direction: attribute in the CSS file then used for? > Shouldn't > > we > > > automatically set that attribute via a [dir=rtl] selector somewhere in a > > shared > > > CSS file? > > this pattern is common: > > default styles { > make a box anchored to the left > } > > if rtl { > anchor it to the right by flipping some mutually exclusive styles > } > > [dir='rtl'] is our current way to do "if rtl", which keys off an explicit HTML > attribute being set. > > I don't know a way this can be accomplished via CSS only (as I don't know if CSS > has the ability to derive the computed direction, just key off of stuff set by > JS or HTML, e.g. classes and attributes). We could still do: [dir='rtl'] { direction: rtl; } Right? Show quoted text > > > 3. For RTL text, the layout will still be somewhat prone to initial > flicker, > > > right? Is there a plan to get rid of that as well? > > I'm unsure of how to fix flicker with our current approach other than > pre-processing. Yeah, that's unfortunate. So, if we need to do preprocessing, and we need to set an attribute on the <html> tag, could we do that via preprocessing? Essentially, do a s/^<html>$/<html dir="$direction">/ ? Show quoted text > > > > Jungshik, ping? > Dan Beam 2015/01/22 00:58:48 ^ how does this selector match? Show quoted text On 2015/01/21 22:47:50, Bernhard Bauer wrote: > On 2015/01/21 19:21:41, Dan Beam wrote: > > On 2015/01/14 18:14:23, Bernhard Bauer wrote: > > > On 2015/01/12 15:47:17, Bernhard Bauer wrote: > > > > On 2015/01/10 00:54:04, Jungshik at google wrote: > > > > > You have to keep this because 'dir' attribute in HTML plays a different > > role > > > > > than 'text-direction:' in CSS. In most cases, the former works better. > > > > > > > > > > Anyway, our CSS has a lot of selectors based on HTML's 'dir' ( > > > > body['dir=rtl']) > > > > > so that we should keep 'dir. > > > > > > > > Oh, ok. That does mean a few things though: > > > > > > > > 1. We should remove the fontFamily and fontSize keys from the dictionary > > > > returned by SetFontAndTextDirection(), and then rename it to > > > SetTextDirection(). > > > > There seem to be some cases left though where those keys are used, so > we'll > > > need > > > > to fix those first. > > > > 2. What is the direction: attribute in the CSS file then used for? > > Shouldn't > > > we > > > > automatically set that attribute via a [dir=rtl] selector somewhere in a > > > shared > > > > CSS file? > > > > this pattern is common: > > > > default styles { > > make a box anchored to the left > > } > > > > if rtl { > > anchor it to the right by flipping some mutually exclusive styles > > } > > > > [dir='rtl'] is our current way to do "if rtl", which keys off an explicit HTML > > attribute being set. > > > > I don't know a way this can be accomplished via CSS only (as I don't know if > CSS > > has the ability to derive the computed direction, just key off of stuff set by > > JS or HTML, e.g. classes and attributes). > > We could still do: > > [dir='rtl'] { ^ how does this selector match? Show quoted text > direction: rtl; > } > > Right? > > > > > 3. For RTL text, the layout will still be somewhat prone to initial > > flicker, > > > > right? Is there a plan to get rid of that as well? > > > > I'm unsure of how to fix flicker with our current approach other than > > pre-processing. > > Yeah, that's unfortunate. > > So, if we need to do preprocessing, and we need to set an attribute on the > <html> tag, could we do that via preprocessing? Essentially, do a > s/^<html>$/<html dir="$direction">/ ? that is more or less what we'd want to do, yes Show quoted text > > > > > > > Jungshik, ping? > > > Dan Beam 2015/01/22 01:14:05 to be more specific (and less rhetorical), current Show quoted text On 2015/01/22 00:58:48, Dan Beam wrote: > On 2015/01/21 22:47:50, Bernhard Bauer wrote: > > On 2015/01/21 19:21:41, Dan Beam wrote: > > > On 2015/01/14 18:14:23, Bernhard Bauer wrote: > > > > On 2015/01/12 15:47:17, Bernhard Bauer wrote: > > > > > On 2015/01/10 00:54:04, Jungshik at google wrote: > > > > > > You have to keep this because 'dir' attribute in HTML plays a > different > > > role > > > > > > than 'text-direction:' in CSS. In most cases, the former works better. > > > > > > > > > > > > Anyway, our CSS has a lot of selectors based on HTML's 'dir' ( > > > > > body['dir=rtl']) > > > > > > so that we should keep 'dir. > > > > > > > > > > Oh, ok. That does mean a few things though: > > > > > > > > > > 1. We should remove the fontFamily and fontSize keys from the > dictionary > > > > > returned by SetFontAndTextDirection(), and then rename it to > > > > SetTextDirection(). > > > > > There seem to be some cases left though where those keys are used, so > > we'll > > > > need > > > > > to fix those first. > > > > > 2. What is the direction: attribute in the CSS file then used for? > > > Shouldn't > > > > we > > > > > automatically set that attribute via a [dir=rtl] selector somewhere in a > > > > shared > > > > > CSS file? > > > > > > this pattern is common: > > > > > > default styles { > > > make a box anchored to the left > > > } > > > > > > if rtl { > > > anchor it to the right by flipping some mutually exclusive styles > > > } > > > > > > [dir='rtl'] is our current way to do "if rtl", which keys off an explicit > HTML > > > attribute being set. > > > > > > I don't know a way this can be accomplished via CSS only (as I don't know if > > CSS > > > has the ability to derive the computed direction, just key off of stuff set > by > > > JS or HTML, e.g. classes and attributes). > > > > We could still do: > > > > [dir='rtl'] { > > ^ how does this selector match? to be more specific (and less rhetorical), currently: <html i18n-values="dir:textdirection"> adds dir="rtl" when JavaScript runs and enables [dir='rtl'] selectors. how would only CSS apply RTL-only rules without pre-processing (either at compile or runtime) all our CSS? Show quoted text > > > direction: rtl; > > } > > > > Right? > > > > > > > 3. For RTL text, the layout will still be somewhat prone to initial > > > flicker, > > > > > right? Is there a plan to get rid of that as well? > > > > > > I'm unsure of how to fix flicker with our current approach other than > > > pre-processing. > > > > Yeah, that's unfortunate. > > > > So, if we need to do preprocessing, and we need to set an attribute on the > > <html> tag, could we do that via preprocessing? Essentially, do a > > s/^<html>$/<html dir="$direction">/ ? > > that is more or less what we'd want to do, yes > > > > > > > > > > > Jungshik, ping? > > > > > Bernhard Bauer 2015/01/22 10:12:35 Sorry, I meant moving the `direction: $1;` line fr Show quoted text On 2015/01/22 01:14:05, Dan Beam wrote: > to be more specific (and less rhetorical), currently: > > <html i18n-values="dir:textdirection"> > > adds dir="rtl" when JavaScript runs and enables [dir='rtl'] selectors. > > how would only CSS apply RTL-only rules without pre-processing (either at > compile or runtime) all our CSS? Sorry, I meant moving the `direction: $1;` line from text_defaults.css into a [dir='rtl'] block in a CSS file that is not preprocessed. Dan Beam 2015/01/22 17:08:41 some things I think are clear but I just want to m Show quoted text On 2015/01/22 10:12:35, Bernhard Bauer wrote: > On 2015/01/22 01:14:05, Dan Beam wrote: > > to be more specific (and less rhetorical), currently: > > > > <html i18n-values="dir:textdirection"> > > > > adds dir="rtl" when JavaScript runs and enables [dir='rtl'] selectors. > > > > how would only CSS apply RTL-only rules without pre-processing (either at > > compile or runtime) all our CSS? > > Sorry, I meant moving the `direction: $1;` line from text_defaults.css into a > [dir='rtl'] block in a CSS file that is not preprocessed. some things I think are clear but I just want to make sure: css works by overriding (cascading). so you set up some default styles and then have some overriding styles later based on your environment (which generally changes based on selectors changing, which is generally from JS, HTML, or stuff like :hover). right now an overwhelming majority of chrome users are LTR, so the page layouts are LTR by default. there are some styles that are mutually exclusive (e.g. positioning in CSS is inherently LTR, as it has a top-left origin for its coordinate system). consider: .frobber { right: 100px; } at run-time (currently), we determine that the locale is RTL, so we change the direction of the <html> element by setting dir="rtl" [1][2]. this is done with i18n-values="dir:textdirection" (which you're proposing we remove with your patch). these types of selectors now apply: [dir=rtl] .frobber { right: auto; /* cancels out the previous rule's right property / left: 100px; } for any element where dir=rtl, built-in user-agent styles apply: [dir=rtl] { direction: rtl; } so there's no reason to have this rule in our application's CSS, as the browser already does it for us. text_defaults.css has `direction: $1` to minimize flicker. JS sets the dir="" attribute (which makes the direction correct), but the page can be painted 1+ times before JS kicks in. [1] https://code.google.com/p/chromium/codesearch#chromium/src/ui/webui/resources... [2] https://code.google.com/p/chromium/codesearch#chromium/src/ui/base/webui/web_... Dan Beam* 2015/01/22 17:09:29 which you were Show quoted text On 2015/01/22 17:08:41, Dan Beam wrote: > On 2015/01/22 10:12:35, Bernhard Bauer wrote: > > On 2015/01/22 01:14:05, Dan Beam wrote: > > > to be more specific (and less rhetorical), currently: > > > > > > <html i18n-values="dir:textdirection"> > > > > > > adds dir="rtl" when JavaScript runs and enables [dir='rtl'] selectors. > > > > > > how would only CSS apply RTL-only rules without pre-processing (either at > > > compile or runtime) all our CSS? > > > > Sorry, I meant moving the `direction: $1;` line from text_defaults.css into a > > [dir='rtl'] block in a CSS file that is not preprocessed. > > some things I think are clear but I just want to make sure: > > css works by overriding (cascading). so you set up some default styles and then > have some overriding styles later based on your environment (which generally > changes based on selectors changing, which is generally from JS, HTML, or stuff > like :hover). > > right now an overwhelming majority of chrome users are LTR, so the page layouts > are LTR by default. there are some styles that are mutually exclusive (e.g. > positioning in CSS is inherently LTR, as it has a top-left origin for its > coordinate system). consider: > > .frobber { > right: 100px; > } > > at run-time (currently), we determine that the locale is RTL, so we change the > direction of the <html> element by setting dir="rtl" [1][2]. this is done with > > i18n-values="dir:textdirection" > > (which you're proposing we remove with your patch). these types of selectors which you were Show quoted text > now apply: > > [dir=rtl] .frobber { > right: auto; /* cancels out the previous rule's right property / > left: 100px; > } > > for any element where dir=rtl, built-in user-agent styles apply: > > [dir=rtl] { > direction: rtl; > } > > so there's no reason to have this rule in our application's CSS, as the browser > already does it for us. > > text_defaults.css has `direction: $1` to minimize flicker. JS sets the dir="" > attribute (which makes the direction correct), but the page can be painted 1+ > times before JS kicks in. > > [1] > https://code.google.com/p/chromium/codesearch#chromium/src/ui/webui/resources... > [2] > https://code.google.com/p/chromium/codesearch#chromium/src/ui/base/webui/web_... Bernhard Bauer* 2015/01/22 17:36:17 Gotcha, thanks for the explanation! Once we have Show quoted text On 2015/01/22 17:08:41, Dan Beam wrote: > On 2015/01/22 10:12:35, Bernhard Bauer wrote: > > On 2015/01/22 01:14:05, Dan Beam wrote: > > > to be more specific (and less rhetorical), currently: > > > > > > <html i18n-values="dir:textdirection"> > > > > > > adds dir="rtl" when JavaScript runs and enables [dir='rtl'] selectors. > > > > > > how would only CSS apply RTL-only rules without pre-processing (either at > > > compile or runtime) all our CSS? > > > > Sorry, I meant moving the `direction: $1;` line from text_defaults.css into a > > [dir='rtl'] block in a CSS file that is not preprocessed. > > some things I think are clear but I just want to make sure: > > css works by overriding (cascading). so you set up some default styles and then > have some overriding styles later based on your environment (which generally > changes based on selectors changing, which is generally from JS, HTML, or stuff > like :hover). > > right now an overwhelming majority of chrome users are LTR, so the page layouts > are LTR by default. there are some styles that are mutually exclusive (e.g. > positioning in CSS is inherently LTR, as it has a top-left origin for its > coordinate system). consider: > > .frobber { > right: 100px; > } > > at run-time (currently), we determine that the locale is RTL, so we change the > direction of the <html> element by setting dir="rtl" [1][2]. this is done with > > i18n-values="dir:textdirection" > > (which you're proposing we remove with your patch). these types of selectors > now apply: > > [dir=rtl] .frobber { > right: auto; /* cancels out the previous rule's right property */ > left: 100px; > } > > for any element where dir=rtl, built-in user-agent styles apply: > > [dir=rtl] { > direction: rtl; > } > > so there's no reason to have this rule in our application's CSS, as the browser > already does it for us. > > text_defaults.css has `direction: $1` to minimize flicker. JS sets the dir="" > attribute (which makes the direction correct), but the page can be painted 1+ > times before JS kicks in. > > [1] > https://code.google.com/p/chromium/codesearch#chromium/src/ui/webui/resources... > [2] > https://code.google.com/p/chromium/codesearch#chromium/src/ui/base/webui/web_... Gotcha, thanks for the explanation! Once we have a no-flicker solution for setting dir=rtl, we should be able to remove the CSS declaration.
3 <head>	3 <head>

4 <meta charset="utf-8">	4 <meta charset="utf-8">

5 <meta name="viewport"	5 <meta name="viewport"

6 content="initial-scale=1, minimum-scale=1, width=device-width">	6 content="initial-scale=1, minimum-scale=1, width=device-width">

7 <title i18n-content="tabTitle"></title>	7 <title i18n-content="tabTitle"></title>

8 <link rel="stylesheet" href="interstitial_v2.css">	8 <link rel="stylesheet" href="interstitial_v2.css">

9 <script src="../../../../ui/webui/resources/js/util.js"></script>	9 <script src="../../../../ui/webui/resources/js/util.js"></script>

10 <script src="ssl.js"></script>	10 <script src="ssl.js"></script>

11 <script src="safe_browsing.js"></script>	11 <script src="safe_browsing.js"></script>

12 <script src="interstitial_v2.js"></script>	12 <script src="interstitial_v2.js"></script>

13 </head>	13 </head>

14 <!-- TODO(jshin): Get rid of i18n-values and import default_text.css instead	14 <body id="body">

15 once interstitial page loading does not use data url any more. -->

16 <body id="body" i18n-values=".style.fontFamily:fontfamily;.style.fontSize:fontsi ze">

17 <div class="interstitial-wrapper">	15 <div class="interstitial-wrapper">

18 <div class="icon" id="icon"></div>	16 <div class="icon" id="icon"></div>

19 <div id="main-message">	17 <div id="main-message">

20 <h1 i18n-content="heading"></h1>	18 <h1 i18n-content="heading"></h1>

21 <p i18n-values=".innerHTML:primaryParagraph"></p>	19 <p i18n-values=".innerHTML:primaryParagraph"></p>

22 </div>	20 </div>

23 <div id="malware-opt-in" class="hidden">	21 <div id="malware-opt-in" class="hidden">

24 <div class="styled-checkbox">	22 <div class="styled-checkbox">

25 <input type="checkbox" id="opt-in-checkbox">	23 <input type="checkbox" id="opt-in-checkbox">

26 <label for="opt-in-checkbox"></label>	24 <label for="opt-in-checkbox"></label>

27 </div>	25 </div>

28 <div id="opt-in-label"></div>	26 <div id="opt-in-label"></div>

29 </div>	27 </div>

30 <div class="nav-wrapper">	28 <div class="nav-wrapper">

31 <button i18n-content="primaryButtonText" id="primary-button"></button>	29 <button i18n-content="primaryButtonText" id="primary-button"></button>

32 <a href="#" id="details-button" class="small-link"	30 <a href="#" id="details-button" class="small-link"

33 i18n-content="openDetails"></a>	31 i18n-content="openDetails"></a>

34 </div>	32 </div>

35 <div id="details" class="hidden">	33 <div id="details" class="hidden">

36 <p i18n-values=".innerHTML:explanationParagraph"></p>	34 <p i18n-values=".innerHTML:explanationParagraph"></p>

37 <p i18n-values=".innerHTML:finalParagraph" id="final-paragraph"></p>	35 <p i18n-values=".innerHTML:finalParagraph" id="final-paragraph"></p>

38 </div>	36 </div>

39 <div id="debugging">	37 <div id="debugging">

40 <p id="error-code"></p>	38 <p id="error-code"></p>

41 <div id="error-debugging-info" class="hidden"></div>	39 <div id="error-debugging-info" class="hidden"></div>

42 </div>	40 </div>

43 </div>	41 </div>

44 </body>	42 </body>

45 </html>	43 </html>

OLD	NEW