launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06606
[Merge] lp:~wgrant/launchpad/html-wtf into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/html-wtf into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/html-wtf/+merge/96265
This branch fixes some invalid HTML in the base template, with a convenient improvement as fallout.
- Fixed the size attribute of the front page search box from "25%" to "25", since it's not a percentage. Browsers just ignored the %.
- Removed the empty <noscript> from base-layout.pt. This was invalid markup which caused mechanise to fail to detect <meta http-equiv="Refresh"> elements, so two apport bug filing tests needed adjusting to prevent the redirect.
- Fixed the HTML for the debug timeline, and moved the conditional up a level to stop the anchor from being rendered even when the timeline was not meant to be shown.
- Made the debug timeline valid by moving it into the body. This brought it up to before the summarize_requests comment, causing the UserRequestOops ID to never show up in the comment. I adjusted the UserRequestOops code to function correctly when called multiple times, which has the handy side-effect of showing the OOPS ID in the visible_render_time.
--
https://code.launchpad.net/~wgrant/launchpad/html-wtf/+merge/96265
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/html-wtf into lp:launchpad.
=== modified file 'lib/lp/app/templates/base-layout-macros.pt'
--- lib/lp/app/templates/base-layout-macros.pt 2012-02-15 22:09:43 +0000
+++ lib/lp/app/templates/base-layout-macros.pt 2012-03-06 23:55:23 +0000
@@ -474,25 +474,24 @@
<metal:debug-timeline define-macro="debug-timeline">
- <a name="debug_timeline"
- id="debug_timeline"
- class="hidden"
- >
- <table
- tal:condition="request/features/visible_render_time"
- tal:define="timeline_actions modules/lp.services.webapp.adapter/get_timeline_actions"
- class="debug-timeline listing"
- >
+ <a tal:condition="request/features/visible_render_time"
+ tal:define="timeline_actions modules/lp.services.webapp.adapter/get_timeline_actions"
+ id="debug_timeline" class="hidden">
+ <table class="debug-timeline listing">
<thead>
- <th>Duration</th>
- <th>Action</th>
+ <tr>
+ <th>Duration</th>
+ <th>Action</th>
+ </tr>
</thead>
- <tr tal:repeat="action timeline_actions">
- <td class="amount" tal:content="action/duration/fmt:millisecondduration"/>
- <td style="font-family: monospace; text-align: left;">
- <pre class="wrap"><span class="action-category" tal:content="action/category"/>: <span class="action-details" tal:content="action/detail"/></pre>
- </td>
- </tr>
+ <tbody>
+ <tr tal:repeat="action timeline_actions">
+ <td class="amount" tal:content="action/duration/fmt:millisecondduration"/>
+ <td style="font-family: monospace; text-align: left;">
+ <pre class="wrap"><span class="action-category" tal:content="action/category"/>: <span class="action-details" tal:content="action/detail"/></pre>
+ </td>
+ </tr>
+ </tbody>
</table>
</a>
</metal:debug-timeline>
=== modified file 'lib/lp/app/templates/base-layout.pt'
--- lib/lp/app/templates/base-layout.pt 2012-02-24 04:54:53 +0000
+++ lib/lp/app/templates/base-layout.pt 2012-03-06 23:55:23 +0000
@@ -43,15 +43,6 @@
html, body {background-image: url(/@@/demo) !important;}
</style>
- <tal:comment condition="nothing">
- Removing the below <noscript></noscript> snippet makes two tests fail
- with redirection problems:
- xx-ubuntu-filebug.txt
- xx-bug-reporting-tools.txt
- No time to look into it now, so keeping it in. XXX Danilo 20110715
- </tal:comment>
- <noscript></noscript>
-
<tal:view condition="not: view/macro:is-page-contentless">
<meta name="description"
tal:condition="view/page_description | nothing"
@@ -176,6 +167,21 @@
<metal:lp-client-cache
use-macro="context/@@+base-layout-macros/lp-client-cache" />
+ <metal:debug-timeline
+ use-macro="context/@@+base-layout-macros/debug-timeline" />
+ <tal:comment
+ tal:condition="request/features/visible_render_time"
+ define="render_time modules/lp.services.webapp.adapter/summarize_requests;"
+ replace='structure string:<script type="text/javascript">
+ var render_time = "${render_time}";
+ LPJS.use("node", "lp.ajax_log" , function(Y) {
+ Y.on("domready", function() {
+ var node = Y.one("#rendertime");
+ node.set("innerHTML", render_time);
+ var ajax_log = new Y.lp.ajax_log();
+ });
+ });
+ </script>' />
</body>
<tal:template>
@@ -196,24 +202,5 @@
-->" />
</tal:template>
-
-
-<metal:debug-timeline
- use-macro="context/@@+base-layout-macros/debug-timeline" />
-
-<tal:comment
- tal:condition="request/features/visible_render_time"
- define="render_time modules/lp.services.webapp.adapter/summarize_requests;"
- replace='structure string:<script type="text/javascript">
- var render_time = "${render_time}";
- LPJS.use("node", "lp.ajax_log" , function(Y) {
- Y.on("domready", function() {
- var node = Y.one("#rendertime");
- node.set("innerHTML", render_time);
- var ajax_log = new Y.lp.ajax_log();
- });
- });
-</script>' />
-
</html>
</metal:page>
=== modified file 'lib/lp/app/templates/root-index.pt'
--- lib/lp/app/templates/root-index.pt 2012-02-24 04:44:55 +0000
+++ lib/lp/app/templates/root-index.pt 2012-03-06 23:55:23 +0000
@@ -136,7 +136,7 @@
xml:lang="en" lang="en" dir="ltr"
tal:attributes="action string:${rooturl}+search"
method="get" accept-charset="UTF-8">
- <input id="text" type="text" name="field.text" size="25%" />
+ <input id="text" type="text" name="field.text" size="25" />
<input id="search" type="submit" value="Search Launchpad" />
</form>
<script type="text/javascript">
=== modified file 'lib/lp/bugs/stories/guided-filebug/xx-bug-reporting-tools.txt'
--- lib/lp/bugs/stories/guided-filebug/xx-bug-reporting-tools.txt 2011-12-29 05:29:36 +0000
+++ lib/lp/bugs/stories/guided-filebug/xx-bug-reporting-tools.txt 2012-03-06 23:55:23 +0000
@@ -61,22 +61,26 @@
The most common case will be that the user is sent to the guided
+filebug page and the user goes through the workflow there.
- >>> filebug_url = (
- ... 'http://launchpad.dev/ubuntu/+source/mozilla-firefox/+filebug/'
- ... '%s' % blob_token)
- >>> user_browser.open(filebug_url)
+ >>> filebug_host = 'launchpad.dev'
+ >>> filebug_path = (
+ ... '/ubuntu/+source/mozilla-firefox/+filebug/%s' % blob_token)
+ >>> filebug_url = 'http://%s%s' % (filebug_host, filebug_path)
+ >>> contents = str(http(
+ ... "GET %s HTTP/1.1\nHostname: %s\n"
+ ... "Authorization: Basic test@xxxxxxxxxxxxx:test\n\n"
+ ... % (filebug_path, filebug_host)))
At first, the user will be shown a message telling them that the extra
data is being processed.
- >>> for message in find_tags_by_class(user_browser.contents, 'message'):
+ >>> for message in find_tags_by_class(contents, 'message'):
... print message.renderContents()
Please wait while bug data is processed. This page will refresh
every 10 seconds until processing is complete.
The page header contains a 10-second meta refresh tag.
- >>> '<meta http-equiv="refresh" content="10"' in user_browser.contents
+ >>> '<meta http-equiv="refresh" content="10"' in contents
True
Once the data has been processed, the +filebug process can continue as
=== modified file 'lib/lp/bugs/stories/guided-filebug/xx-ubuntu-filebug.txt'
--- lib/lp/bugs/stories/guided-filebug/xx-ubuntu-filebug.txt 2011-12-29 05:29:36 +0000
+++ lib/lp/bugs/stories/guided-filebug/xx-ubuntu-filebug.txt 2012-03-06 23:55:23 +0000
@@ -71,6 +71,14 @@
... extra_filebug_data, 'not/important', 'not.important')
>>> anon_browser.getControl(name='FORM_SUBMIT').click()
>>> blob_token = anon_browser.headers['X-Launchpad-Blob-Token']
+ >>> from lp.bugs.interfaces.apportjob import IProcessApportBlobJobSource
+ >>> login('foo.bar@xxxxxxxxxxxxx')
+ >>> job = getUtility(IProcessApportBlobJobSource).getByBlobUUID(
+ ... blob_token)
+ >>> job.job.start()
+ >>> job.run()
+ >>> job.job.complete()
+ >>> logout()
>>> filebug_url = (
... 'http://launchpad.dev/ubuntu/+source/mozilla-firefox/+filebug/'
... '%s' % blob_token)
=== modified file 'lib/lp/services/webapp/adapter.py'
--- lib/lp/services/webapp/adapter.py 2011-12-30 08:13:14 +0000
+++ lib/lp/services/webapp/adapter.py 2012-03-06 23:55:23 +0000
@@ -226,11 +226,11 @@
timeline = get_request_timeline(request)
from lp.services.webapp.errorlog import (
maybe_record_user_requested_oops)
- oopsid = maybe_record_user_requested_oops()
- if oopsid is None:
+ maybe_record_user_requested_oops()
+ if request.oopsid is None:
oops_str = ""
else:
- oops_str = " %s" % oopsid
+ oops_str = " %s" % request.oopsid
log = "%s queries/external actions issued in %.2f seconds%s" % (
len(timeline.actions), secs, oops_str)
return log
=== modified file 'lib/lp/services/webapp/errorlog.py'
--- lib/lp/services/webapp/errorlog.py 2012-01-01 02:58:52 +0000
+++ lib/lp/services/webapp/errorlog.py 2012-03-06 23:55:23 +0000
@@ -542,18 +542,16 @@
def maybe_record_user_requested_oops():
"""If an OOPS has been requested, report one.
- :return: The oopsid of the requested oops. Returns None if an oops was
- not requested, or if there is already an OOPS.
+ It will be stored in request.oopsid.
"""
request = get_current_browser_request()
- # If there is no request, or there is an oops already, then return.
- if (request is None or
- request.oopsid is not None or
- not request.annotations.get(LAZR_OOPS_USER_REQUESTED_KEY, False)):
- return None
- globalErrorUtility.raising(
- (UserRequestOops, UserRequestOops(), None), request)
- return request.oopsid
+ # If there's a request and no existing OOPS, but an OOPS has been
+ # requested, record one.
+ if (request is not None
+ and request.oopsid is None
+ and request.annotations.get(LAZR_OOPS_USER_REQUESTED_KEY, False)):
+ globalErrorUtility.raising(
+ (UserRequestOops, UserRequestOops(), None), request)
class OopsNamespace(view):
=== modified file 'lib/lp/services/webapp/tests/test_user_requested_oops.py'
--- lib/lp/services/webapp/tests/test_user_requested_oops.py 2012-01-01 02:58:52 +0000
+++ lib/lp/services/webapp/tests/test_user_requested_oops.py 2012-03-06 23:55:23 +0000
@@ -37,20 +37,28 @@
def test_none_requested(self):
# If an oops was not requested, then maybe_record_user_requested_oops
# does not record an oops.
- oops_id = maybe_record_user_requested_oops()
- self.assertIs(None, oops_id)
request = get_current_browser_request()
+ maybe_record_user_requested_oops()
self.assertIs(None, request.oopsid)
def test_annotation_key(self):
- # The request for an oops is stored in the request annotations. If a
- # user request oops is recorded, the oops id is returned, and also
- # stored in the request.
- request = get_current_browser_request()
- request.annotations[LAZR_OOPS_USER_REQUESTED_KEY] = True
- oops_id = maybe_record_user_requested_oops()
- self.assertIsNot(None, oops_id)
- self.assertEqual(oops_id, request.oopsid)
+ # The request for an oops is stored in the request annotations.
+ # If a user request oops is recorded, the oops id is stored in
+ # the request.
+ request = get_current_browser_request()
+ request.annotations[LAZR_OOPS_USER_REQUESTED_KEY] = True
+ self.assertIs(None, request.oopsid)
+ maybe_record_user_requested_oops()
+ self.assertIsNot(None, request.oopsid)
+
+ def test_multiple_calls(self):
+ # Asking to record the OOPS twice just returns the same ID.
+ request = get_current_browser_request()
+ request.annotations[LAZR_OOPS_USER_REQUESTED_KEY] = True
+ maybe_record_user_requested_oops()
+ orig_oops_id = request.oopsid
+ maybe_record_user_requested_oops()
+ self.assertEqual(orig_oops_id, request.oopsid)
def test_existing_oops_stops_user_requested(self):
# If there is already an existing oops id in the request, then the
@@ -58,8 +66,7 @@
request = get_current_browser_request()
request.oopsid = "EXISTING"
request.annotations[LAZR_OOPS_USER_REQUESTED_KEY] = True
- oops_id = maybe_record_user_requested_oops()
- self.assertIs(None, oops_id)
+ maybe_record_user_requested_oops()
self.assertEqual("EXISTING", request.oopsid)
def test_OopsNamespace_traverse(self):
