Launchpad itself

Merge lp:~edwin-grubbs/launchpad/bug-401748-ie8-new-project into lp:launchpad/db-devel

  1. bug-401748-ie8-new-project
  2. Merge into db-devel
Proposed by Edwin Grubbs
Status: Merged
Approved by: Graham Binns
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~edwin-grubbs/launchpad/bug-401748-ie8-new-project
Merge into: lp:launchpad/db-devel
Diff against target: None lines
To merge this branch: bzr merge lp:~edwin-grubbs/launchpad/bug-401748-ie8-new-project
Reviewer Review Type Date Requested Status
Francis J. Lacoste (community) release-critical Approve
Graham Binns (community) Approve
Review via email: mp+9083@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Summary
-------

Fixed problem creating new launchpad projects in IE8.

Implementation details
----------------------

IE8 throws an exception when you try to focus a hidden element.

The help pane's Continue button was also in the middle of the page
because IE8 has a problem with visibility:inherit.

The slider effect was not working on the various sections of licenses
since IE8 doesn't respond to setting the table's height to zero.
Wrapped it in a div.

Fixed a fragile test by adding sleep between actions.

Tests
-----

The bugs only affect IE8, so the tests, which are run in firefox,
don't prove anything except that there were no regressions.

./bin/lp-windmill test=lib/canonical/launchpad/windmill/tests/test_registry/test_plusnew_step1.py firefox http://launchpad.dev:8085
./bin/lp-windmill test=lib/canonical/launchpad/windmill/tests/test_registry/test_plusnew_step2.py firefox http://launchpad.dev:8085

Demo and Q/A
------------

 * Open http://launchpad.dev/projects/+new
    * Enter "firefox", "firefox2", "foo", "bar" and click "Continue".
    * Click on "No, this is a new project."
    * You should see a whole form expand, and the links above the license
      sections should hide/show each section.

Revision history for this message
Graham Binns (gmb) :
review: Approve
Revision history for this message
Francis J. Lacoste (flacoste) wrote :

On the condition of remoing the alert call, use Y.log for that.

review: Approve (release-critical)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/javascript/lp/lp.js'
2--- lib/canonical/launchpad/javascript/lp/lp.js 2009-07-13 10:52:46 +0000
3+++ lib/canonical/launchpad/javascript/lp/lp.js 2009-07-20 23:17:14 +0000
4@@ -516,7 +516,15 @@
5 for (var i = 0; i < nodes.length; i++) {
6 var node = nodes[i];
7 if (node.focus) {
8- node.focus();
9+ try {
10+ // Trying to focus a hidden element throws an error in IE8.
11+ if (node.offsetHeight !== 0) {
12+ node.focus();
13+ }
14+ } catch (e) {
15+ alert('In setFocusByName(<' +
16+ node.tagName + ' type=' + node.type + '>): ' + e);
17+ }
18 break;
19 }
20 }
21
22=== modified file 'lib/canonical/launchpad/windmill/tests/test_registry/test_plusnew_step2.py'
23--- lib/canonical/launchpad/windmill/tests/test_registry/test_plusnew_step2.py 2009-06-25 05:30:52 +0000
24+++ lib/canonical/launchpad/windmill/tests/test_registry/test_plusnew_step2.py 2009-07-21 03:56:03 +0000
25@@ -58,6 +58,7 @@
26 validator='className|lazr-opened')
27 # Clicking it again hides the results.
28 client.click(id='search-results-expander')
29+ client.waits.sleep(milliseconds=u'500')
30 client.asserts.assertProperty(
31 id=u'search-results',
32 validator='className|lazr-closed')
33
34=== modified file 'lib/canonical/widgets/product.py'
35--- lib/canonical/widgets/product.py 2009-07-17 00:26:05 +0000
36+++ lib/canonical/widgets/product.py 2009-07-21 03:56:03 +0000
37@@ -362,7 +362,9 @@
38 return self.template()
39
40 def _renderTable(self, category, column_count=1):
41- html = ['<table id="%s">' % category]
42+ # The tables are wrapped in divs, since IE8 does not respond
43+ # to setting the table's height to zero.
44+ html = ['<div id="%s"><table>' % category]
45 rendered_items = self.items_by_category[category]
46 row_count = int(math.ceil(len(rendered_items) / float(column_count)))
47 for i in range(0, row_count):
48@@ -373,7 +375,7 @@
49 break
50 html.append('<td>%s</td>' % rendered_items[index])
51 html.append('</tr>')
52- html.append('</table>')
53+ html.append('</table></div>')
54 return '\n'.join(html)
55
56
57
58=== modified file 'lib/lp/services/inlinehelp/javascript/inlinehelp.js'
59--- lib/lp/services/inlinehelp/javascript/inlinehelp.js 2009-06-30 21:06:27 +0000
60+++ lib/lp/services/inlinehelp/javascript/inlinehelp.js 2009-07-20 23:17:14 +0000
61@@ -24,8 +24,10 @@
62 page elements.
63 */
64 // The button is inserted in the page dynamically:
65+ // Changed from an <input type=button> to a <button> since
66+ // IE8 doesn't handle style.css's input{visibility:inherit} correctly.
67 $('help-close').innerHTML =
68- '<input id="help-close-btn" type="button" value="Continue">';
69+ '<button id="help-close-btn" value="Continue">';
70 forEach(findHelpLinks(), setupHelpTrigger);
71 initHelpPane();
72 }

Subscribers

People subscribed via source and target branches

to status/vote changes: