Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Delta Between Two Patch Sets: notification.html

Issue 6098518317989888: Fix: Notification popup is broken (Closed)
Left Patch Set: Changes after Thomas notes Created Feb. 18, 2014, 10:13 a.m.
Right Patch Set: Change back activeNotification unset Created Feb. 26, 2014, 6:31 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« chrome/ext/background.js ('K') | « metadata.chrome ('k') | no next file » | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 <!DOCTYPE html> 1 <!DOCTYPE html>
2 <html> 2 <html>
3 <head> 3 <head>
4 <style> 4 <style>
5 body 5 body
6 { 6 {
7 background-color: #f8f6f2; 7 background-color: #f8f6f2;
8 font-family: Arial, sans; 8 font-family: Arial, sans;
9 font-size: 12px; 9 font-size: 12px;
10 } 10 }
11 11
12 #notification 12 #notification
13 { 13 {
14 position: absolute; 14 position: absolute;
15 top: 5px; 15 top: 5px;
16 left: 65px; 16 left: 65px;
17 } 17 }
18 18
19 #notification>h1 19 #notification>h1
20 { 20 {
21 font-size: 16px; 21 font-size: 16px;
22 } 22 }
23 23
24 #notification>* 24 #notification>*
25 { 25 {
26 margin: 5px; 26 margin: 5px;
27 } 27 }
28 </style> 28 </style>
29 <script src="ext/common.js"></script> 29 <script src="ext/common.js"></script>
Thomas Greiner 2014/02/18 14:26:17 Well spotted. However, notifications.js also makes
saroyanm 2014/02/18 16:20:28 Good point.
30 <script src="ext/background.js"></script>
Felix Dahlke 2014/02/27 14:14:43 Guess I'm missing something, but what is this nece
Thomas Greiner 2014/02/27 15:10:37 This fixes notification.js which depends on ext.*
Sebastian Noack 2014/02/28 09:56:03 No, background.js must be only executed once in th
saroyanm 2014/02/28 10:56:12 Not sure why the things will be broken, actually w
Sebastian Noack 2014/02/28 12:35:03 This will run code twice, that is supposed to run
saroyanm 2014/02/28 13:47:04 Yes I agree, but it will run twice for the users w
Sebastian Noack 2014/02/28 14:23:49 I've just realized that webkitNotification.createH
saroyanm 2014/02/28 14:34:25 As I've mentioned in my previous comment It's depr
Sebastian Noack 2014/02/28 16:59:22 I'm very much in the favor of getting rid of that
Sebastian Noack 2014/02/28 17:01:00 s/if worse/is worth/
saroyanm 2014/03/03 07:08:51 Sebastian I see your point and yes while It's not
Thomas Greiner 2014/03/04 09:28:15 We should not remove the case for createHTMLNotifi
Sebastian Noack 2014/03/05 10:45:49 createHTMLNotification isn't available for any ver
Thomas Greiner 2014/03/05 11:30:51 It's still better than window.confirm and it can h
saroyanm 2014/03/05 11:33:25 chrome.notifications Are stable since chrome v28 s
30 <script src="notification.js"></script> 31 <script src="notification.js"></script>
31 </head> 32 </head>
32 <body> 33 <body>
33 <img src="icons/abp-48.png"> 34 <img src="icons/abp-48.png">
34 <div id="notification"> 35 <div id="notification">
35 <h1 id="notification-title"></h1> 36 <h1 id="notification-title"></h1>
36 <p id="notification-message"></p> 37 <p id="notification-message"></p>
37 </div> 38 </div>
38 </body> 39 </body>
39 </html> 40 </html>
LEFTRIGHT

Powered by Google App Engine
This is Rietveld