launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #07986
[Merge] lp:~sinzui/launchpad/bug-listings-urls into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/bug-listings-urls into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #939195 in Launchpad itself: "cannot order bugs by 'Date last updated' in IE9"
https://bugs.launchpad.net/launchpad/+bug/939195
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/bug-listings-urls/+merge/106266
Pre-implementation: wgrant
Bug listings are very broken in MSIE because calls to
preserve history corrupt the URLs and state of the page. Users cannot
see more than one batch nor can they sort the listing.
--------------------------------------------------------------------
RULES
* We had agreed to disable the history module for MSIE and Opera.
I tested the latest version of Opera and it does work contrary to
reports that bug listings were broken. The History module has
a lot of improvements in YUI 3.5 so I decided to look into to
the double pillar in the URL. My suspicious were confirmed that
IE was getting a relative URL when I compared IE and Chromium in
their debuggers.
* Ensure pathname are absolute.
QA
Using MSIE and Opera
* Visit https://bugs.qastaging.launchpad.net/pocket-lint
* Verify you can sort the listing (No error dialog).
* Verify you can see the next page (No error dialog).
LINT
lib/lp/app/javascript/lp.js
lib/lp/app/javascript/tests/test_lp.html
lib/lp/app/javascript/tests/test_lp.js
TEST
./bin/test -vvc --layer=YUITest
IMPLEMENTATION
I fixed a helper method that assumes the browser returns a same pathname,
which IE does not. I added a rule to ensure a double slash was not
created because I recall we have seen this in the past.
lib/lp/app/javascript/lp.js
lib/lp/app/javascript/tests/test_lp.html
lib/lp/app/javascript/tests/test_lp.js
--
https://code.launchpad.net/~sinzui/launchpad/bug-listings-urls/+merge/106266
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/bug-listings-urls into lp:launchpad.
=== modified file 'lib/lp/app/javascript/lp.js'
--- lib/lp/app/javascript/lp.js 2012-03-20 03:01:07 +0000
+++ lib/lp/app/javascript/lp.js 2012-05-17 21:29:18 +0000
@@ -77,7 +77,16 @@
* Return the path portion of the specified URL.
*/
Y.lp.get_url_path = function(url) {
- return get_hyperlink(url).get('pathname');
+ pathname = get_hyperlink(url).get('pathname');
+ if (!pathname || pathname[0] !== '/') {
+ // Ensure the leading slash often dropped by msie.
+ pathname = '/' + pathname;
+ }
+ if (pathname.length > 1 && pathname[1] === '/') {
+ // Ensure a single root often broken by a concatenation error.
+ pathname = pathname.substring(1, pathname.length);
+ }
+ return pathname;
};
/**
=== added file 'lib/lp/app/javascript/tests/test_lp.html'
--- lib/lp/app/javascript/tests/test_lp.html 1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/tests/test_lp.html 2012-05-17 21:29:18 +0000
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<!--
+Copyright 2012 Canonical Ltd. This software is licensed under the
+GNU Affero General Public License version 3 (see the file LICENSE).
+-->
+<html>
+ <head>
+ <title>Test lp-names</title>
+ <!-- YUI and test setup -->
+ <script type="text/javascript"
+ src="../../../../../build/js/yui/yui/yui.js">
+ </script>
+ <link rel="stylesheet"
+ href="../../../../../build/js/yui/console/assets/console-core.css" />
+ <link rel="stylesheet"
+ href="../../../../../build/js/yui/console/assets/skins/sam/console.css" />
+ <link rel="stylesheet"
+ href="../../../../../build/js/yui/test/assets/skins/sam/test.css" />
+
+ <script type="text/javascript"
+ src="../../../../../build/js/lp/app/testing/testrunner.js"></script>
+ <link rel="stylesheet" href="../../../app/javascript/testing/test.css" />
+
+ <!-- The module under test. -->
+ <script type="text/javascript" src="../lp.js"></script>
+
+ <!-- The test suite. -->
+ <script type="text/javascript" src="test_lp.js"></script>
+
+ </head>
+ <body class="yui3-skin-sam">
+ <ul id="suites">
+ <li>lp.test</li>
+ </ul>
+
+ <div id="fixture"></div>
+ </body>
+</html>
=== added file 'lib/lp/app/javascript/tests/test_lp.js'
--- lib/lp/app/javascript/tests/test_lp.js 1970-01-01 00:00:00 +0000
+++ lib/lp/app/javascript/tests/test_lp.js 2012-05-17 21:29:18 +0000
@@ -0,0 +1,26 @@
+YUI().use('lp.testing.runner', 'test', 'console', 'node', 'lp', function(Y) {
+
+ var suite = new Y.Test.Suite("lp helper tests");
+
+ suite.add(new Y.Test.Case({
+ name: 'test_bugs_branches',
+
+ test_get_url_path_with_pillar: function () {
+ Y.Assert.areEqual(
+ '/fnord', Y.lp.get_url_path('http://launchpad.dev/fnord'));
+ },
+
+ test_get_url_path_without_slash: function () {
+ Y.Assert.areEqual(
+ '/', Y.lp.get_url_path('http://launchpad.dev'));
+ },
+
+ test_get_url_path_with_double_slash: function () {
+ Y.Assert.areEqual(
+ '/fnord', Y.lp.get_url_path('http://launchpad.dev//fnord'));
+ }
+ }));
+
+ Y.lp.testing.Runner.run(suite);
+});
+
Follow ups
