chrome/browser/resources/safe_browsing/malware_block_v2.html - Issue 10855260: Safe Browsing malware interstitial redesign field trial.

Side by Side Diff: chrome/browser/resources/safe_browsing/malware_block_v2.html

Issue 10855260: Safe Browsing malware interstitial redesign field trial. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src

Patch Set: webui style fixes Created 8 years, 4 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 | Annotate | Revision Log

« chrome/browser/resources/safe_browsing/malware_block_v2.css ('K') | « chrome/browser/resources/safe_browsing/malware_block_v2.css ('k') | chrome/browser/resources/safe_browsing/malware_block_v2.js » ('j') | chrome/browser/resources/safe_browsing/malware_block_v2.js » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <html id="template_root" i18n-values="dir:textdirection">
	arv (Not doing code reviews) 2012/08/22 14:33:45 template-root template-root mattm 2012/08/22 20:33:47 Done. Show quoted text On 2012/08/22 14:33:45, arv wrote: > template-root Done.
	3 <head>

	4 <meta charset="utf-8">

	5 <title i18n-content="title"></title>

	6 <link rel="stylesheet" href="malware_block_v2.css">

	7 <script src="../shared/js/util.js"></script>

	8 <script src="malware_block_v2.js"></script>

	9 </head>

	10 <body>

	11 <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 you really need a table for this? also, indent <table>, IMO arv (Not doing code reviews) 2012/08/22 14:33:45 Useless table also, no presentational attributes Useless table also, no presentational attributes please mattm 2012/08/22 20:33:47 Do you mind if I leave the table for now? This is Show quoted text On 2012/08/22 02:34:03, Dan Beam wrote: > you really need a table for this? Do you mind if I leave the table for now? This is all just based off the previous version of this page, which you can see was not style compliant. I can try to get rid of this table but I'd rather not block the change on it. Show quoted text > also, indent <table>, IMO done.
	12 <td class="cell" valign="middle" align="center">

	13 <div class="box">

	14 <div><img
	Dan Beam 2012/08/22 02:34:03 \n before <img> \n before <img> arv (Not doing code reviews) 2012/08/22 14:33:45 Is this div needed? Is this div needed? mattm 2012/08/22 20:33:47 Done. Show quoted text On 2012/08/22 02:34:03, Dan Beam wrote: > \n before <img> Done. mattm 2012/08/22 20:33:47 Done. Show quoted text On 2012/08/22 14:33:45, arv wrote: > Is this div needed? Done.
	15 src="../../../app/theme/%DISTRIBUTION%/product_logo_name_108.png"

	16 class="logo"></div>

	17

	18 <div id=headline i18n-content="headLine"></div>

	19 <div class="float"><img src="images/malware_icon_v2.jpg"
	arv (Not doing code reviews) 2012/08/22 14:33:45 Useless div? Useless div? mattm 2012/08/22 20:33:47 Done. Show quoted text On 2012/08/22 14:33:45, arv wrote: > Useless div? Done.
	20 width=200 height=200 alt="Malware Icon" /></div>

	21 <div class="main" i18n-values=".innerHTML:description1"></div>
	arv (Not doing code reviews) 2012/08/22 14:33:45 Do you really need innerHTML? Do you really need innerHTML? mattm 2012/08/22 20:33:47 Done. Show quoted text On 2012/08/22 14:33:45, arv wrote: > Do you really need innerHTML? Done.
	22 <div class="main" i18n-content="description2"></div>

	23 <div class="main">

	24 <span i18n-content="description3"></span>

	25 <a id="learn-more-link" href="#" i18n-content="learnMore"></a>

	26 </div>

	27

	28 <div class="main">

	29 <form class="submission">
	arv (Not doing code reviews) 2012/08/22 14:33:45 Why form? Why form? mattm 2012/08/22 20:33:47 Done. Show quoted text On 2012/08/22 14:33:45, arv wrote: > Why form? Done.
	30 <button id="back" i18n-content="back_button"></button>

	31 </form>

	32 </div>

	33

	34 <div class="clear"></div>

	35

	36 <div id="see-more-contents" hidden>

	37 <div class="dropdown" i18n-values=".innerHTML:description4"></div>

	38 <div class="dropdown">

	39 <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' Don't use # links Use a button if the link doesn't take the user anywhere mattm 2012/08/22 20:33:47 This link does actually take you somewhere, it's j Show quoted text On 2012/08/22 14:33:45, arv wrote: > Don't use # links > > Use a button if the link doesn't take the user anywhere This link does actually take you somewhere, it's just handled by sending a command to the C++ handler due to how interstitial pages work. (only the "more" / "less" link doesn't go somewhere. But that is the UI that was decided on, and it's been through UI review. You can ask Yi if you have additional questions.)
	40 jsdisplay="displayShowDiagnostic"></a>
	arv (Not doing code reviews) 2012/08/22 14:33:45 I would remove the use of jsdisplay and then remov I would remove the use of jsdisplay and then remove jstemplate too. It is really overkill for this page. mattm 2012/08/22 20:33:47 Doesn't jstemplate also do all the i18n strings an Show quoted text On 2012/08/22 14:33:45, arv wrote: > I would remove the use of jsdisplay and then remove jstemplate too. It is really > overkill for this page. Doesn't jstemplate also do all the i18n strings and stuff too? Stuffing that all in manually doesn't seem easier.
	41 </div>

	42 </div>

	43

	44 <hr>

	45

	46 <div id="footer">
	arv (Not doing code reviews) 2012/08/22 14:33:45 <footer> <footer> mattm 2012/08/22 20:33:47 Done. Show quoted text On 2012/08/22 14:33:45, arv wrote: > <footer> Done.
	47 <a id="see-more-link" class="float" href="#">

	48 <span id="see-more-text" i18n-content="seeMore"></span>

	49 <span id="see-less-text" i18n-content="seeLess" hidden></span>

	50 </a>

	51 <div class="checkbox" jsdisplay="displaycheckbox">

	52 <label for="check-report">
	arv (Not doing code reviews) 2012/08/22 14:33:45 useless attribute useless attribute mattm 2012/08/22 20:33:47 Done. Show quoted text On 2012/08/22 14:33:45, arv wrote: > useless attribute Done.
	53 <input id="check-report" name="checked" type="checkbox"
	arv (Not doing code reviews) 2012/08/22 14:33:45 Is the name really used? Is the name really used? mattm 2012/08/22 20:33:47 removed Show quoted text On 2012/08/22 14:33:45, arv wrote: > Is the name really used? removed
	54 jsvalues=".checked:boxchecked">
	arv (Not doing code reviews) 2012/08/22 14:33:45 i18n-values i18n-values mattm 2012/08/22 20:33:47 I'm not sure the exact difference, but that doesn' Show quoted text On 2012/08/22 14:33:45, arv wrote: > i18n-values I'm not sure the exact difference, but that doesn't seem right. This is setting whether it's checked by default, it's not internationalization related.
	55 <span i18n-values=".innerHTML:confirm_text"></span>
	arv (Not doing code reviews) 2012/08/22 14:33:45 innerHTML? innerHTML? mattm 2012/08/22 20:33:47 the string contains an embedded link Show quoted text On 2012/08/22 14:33:45, arv wrote: > innerHTML? the string contains an embedded link
	56 </label>

	57 </div>

	58 </div>

	59 </div>

	60 </td>

	61 </table>

	62 </body>

	63 </html>

	64

OLD	NEW