Chromium Code Reviews Chromium Code Reviews
Issue 10855260:
Safe Browsing malware interstitial redesign field trial. (Closed)
Base URL: svn://svn.chromium.org/chrome/trunk/src| Index: chrome/browser/resources/safe_browsing/malware_block_v2.html |
| diff --git a/chrome/browser/resources/safe_browsing/malware_block_v2.html b/chrome/browser/resources/safe_browsing/malware_block_v2.html |
| new file mode 100644 |
| index 0000000000000000000000000000000000000000..426b959b6bcbefc62f4d1c93eb8ae3db3c44b480 |
| --- /dev/null |
| +++ b/chrome/browser/resources/safe_browsing/malware_block_v2.html |
| @@ -0,0 +1,64 @@ |
| +<!DOCTYPE html> |
| +<html id="template_root" i18n-values="dir:textdirection"> |
|
arv (Not doing code reviews)
2012/08/22 14:33:45
template-root
mattm
2012/08/22 20:33:47
Done.
|
| +<head> |
| + <meta charset="utf-8"> |
| + <title i18n-content="title"></title> |
| + <link rel="stylesheet" href="malware_block_v2.css"> |
| + <script src="../shared/js/util.js"></script> |
| + <script src="malware_block_v2.js"></script> |
| +</head> |
| +<body> |
| +<table width="100%" cellspacing="0" cellpadding="0"> |
|
Dan Beam
2012/08/22 02:34:03
you really need a table for this? also, indent <ta
arv (Not doing code reviews)
2012/08/22 14:33:45
Useless table
also, no presentational attributes
mattm
2012/08/22 20:33:47
Do you mind if I leave the table for now? This is
|
| + <td class="cell" valign="middle" align="center"> |
| + <div class="box"> |
| + <div><img |
|
Dan Beam
2012/08/22 02:34:03
\n before <img>
arv (Not doing code reviews)
2012/08/22 14:33:45
Is this div needed?
mattm
2012/08/22 20:33:47
Done.
mattm
2012/08/22 20:33:47
Done.
|
| + src="../../../app/theme/%DISTRIBUTION%/product_logo_name_108.png" |
| + class="logo"></div> |
| + |
| + <div id=headline i18n-content="headLine"></div> |
| + <div class="float"><img src="images/malware_icon_v2.jpg" |
|
arv (Not doing code reviews)
2012/08/22 14:33:45
Useless div?
mattm
2012/08/22 20:33:47
Done.
|
| + width=200 height=200 alt="Malware Icon" /></div> |
| + <div class="main" i18n-values=".innerHTML:description1"></div> |
|
arv (Not doing code reviews)
2012/08/22 14:33:45
Do you really need innerHTML?
mattm
2012/08/22 20:33:47
Done.
|
| + <div class="main" i18n-content="description2"></div> |
| + <div class="main"> |
| + <span i18n-content="description3"></span> |
| + <a id="learn-more-link" href="#" i18n-content="learnMore"></a> |
| + </div> |
| + |
| + <div class="main"> |
| + <form class="submission"> |
|
arv (Not doing code reviews)
2012/08/22 14:33:45
Why form?
mattm
2012/08/22 20:33:47
Done.
|
| + <button id="back" i18n-content="back_button"></button> |
| + </form> |
| + </div> |
| + |
| + <div class="clear"></div> |
| + |
| + <div id="see-more-contents" hidden> |
| + <div class="dropdown" i18n-values=".innerHTML:description4"></div> |
| + <div class="dropdown"> |
| + <a id="show-diagnostic-link" href="#" i18n-content="details" |
|
arv (Not doing code reviews)
2012/08/22 14:33:45
Don't use # links
Use a button if the link doesn'
mattm
2012/08/22 20:33:47
This link does actually take you somewhere, it's j
|
| + jsdisplay="displayShowDiagnostic"></a> |
|
arv (Not doing code reviews)
2012/08/22 14:33:45
I would remove the use of jsdisplay and then remov
mattm
2012/08/22 20:33:47
Doesn't jstemplate also do all the i18n strings an
|
| + </div> |
| + </div> |
| + |
| + <hr> |
| + |
| + <div id="footer"> |
|
arv (Not doing code reviews)
2012/08/22 14:33:45
<footer>
mattm
2012/08/22 20:33:47
Done.
|
| + <a id="see-more-link" class="float" href="#"> |
| + <span id="see-more-text" i18n-content="seeMore"></span> |
| + <span id="see-less-text" i18n-content="seeLess" hidden></span> |
| + </a> |
| + <div class="checkbox" jsdisplay="displaycheckbox"> |
| + <label for="check-report"> |
|
arv (Not doing code reviews)
2012/08/22 14:33:45
useless attribute
mattm
2012/08/22 20:33:47
Done.
|
| + <input id="check-report" name="checked" type="checkbox" |
|
arv (Not doing code reviews)
2012/08/22 14:33:45
Is the name really used?
mattm
2012/08/22 20:33:47
removed
|
| + jsvalues=".checked:boxchecked"> |
|
arv (Not doing code reviews)
2012/08/22 14:33:45
i18n-values
mattm
2012/08/22 20:33:47
I'm not sure the exact difference, but that doesn'
|
| + <span i18n-values=".innerHTML:confirm_text"></span> |
|
arv (Not doing code reviews)
2012/08/22 14:33:45
innerHTML?
mattm
2012/08/22 20:33:47
the string contains an embedded link
|
| + </label> |
| + </div> |
| + </div> |
| + </div> |
| + </td> |
| +</table> |
| +</body> |
| +</html> |
| + |
