Merge lp:~edwin-grubbs/launchpad/bug-401748-ie8-new-project into lp:launchpad/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
=== modified file 'lib/canonical/launchpad/javascript/lp/lp.js'
--- lib/canonical/launchpad/javascript/lp/lp.js 2009-07-13 10:52:46 +0000
+++ lib/canonical/launchpad/javascript/lp/lp.js 2009-07-20 23:17:14 +0000
@@ -516,7 +516,15 @@
516 for (var i = 0; i < nodes.length; i++) {516 for (var i = 0; i < nodes.length; i++) {
517 var node = nodes[i];517 var node = nodes[i];
518 if (node.focus) {518 if (node.focus) {
519 node.focus();519 try {
520 // Trying to focus a hidden element throws an error in IE8.
521 if (node.offsetHeight !== 0) {
522 node.focus();
523 }
524 } catch (e) {
525 alert('In setFocusByName(<' +
526 node.tagName + ' type=' + node.type + '>): ' + e);
527 }
520 break;528 break;
521 }529 }
522 }530 }
523531
=== modified file 'lib/canonical/launchpad/windmill/tests/test_registry/test_plusnew_step2.py'
--- lib/canonical/launchpad/windmill/tests/test_registry/test_plusnew_step2.py 2009-06-25 05:30:52 +0000
+++ lib/canonical/launchpad/windmill/tests/test_registry/test_plusnew_step2.py 2009-07-21 03:56:03 +0000
@@ -58,6 +58,7 @@
58 validator='className|lazr-opened')58 validator='className|lazr-opened')
59 # Clicking it again hides the results.59 # Clicking it again hides the results.
60 client.click(id='search-results-expander')60 client.click(id='search-results-expander')
61 client.waits.sleep(milliseconds=u'500')
61 client.asserts.assertProperty(62 client.asserts.assertProperty(
62 id=u'search-results',63 id=u'search-results',
63 validator='className|lazr-closed')64 validator='className|lazr-closed')
6465
=== modified file 'lib/canonical/widgets/product.py'
--- lib/canonical/widgets/product.py 2009-07-17 00:26:05 +0000
+++ lib/canonical/widgets/product.py 2009-07-21 03:56:03 +0000
@@ -362,7 +362,9 @@
362 return self.template()362 return self.template()
363363
364 def _renderTable(self, category, column_count=1):364 def _renderTable(self, category, column_count=1):
365 html = ['<table id="%s">' % category]365 # The tables are wrapped in divs, since IE8 does not respond
366 # to setting the table's height to zero.
367 html = ['<div id="%s"><table>' % category]
366 rendered_items = self.items_by_category[category]368 rendered_items = self.items_by_category[category]
367 row_count = int(math.ceil(len(rendered_items) / float(column_count)))369 row_count = int(math.ceil(len(rendered_items) / float(column_count)))
368 for i in range(0, row_count):370 for i in range(0, row_count):
@@ -373,7 +375,7 @@
373 break375 break
374 html.append('<td>%s</td>' % rendered_items[index])376 html.append('<td>%s</td>' % rendered_items[index])
375 html.append('</tr>')377 html.append('</tr>')
376 html.append('</table>')378 html.append('</table></div>')
377 return '\n'.join(html)379 return '\n'.join(html)
378380
379381
380382
=== modified file 'lib/lp/services/inlinehelp/javascript/inlinehelp.js'
--- lib/lp/services/inlinehelp/javascript/inlinehelp.js 2009-06-30 21:06:27 +0000
+++ lib/lp/services/inlinehelp/javascript/inlinehelp.js 2009-07-20 23:17:14 +0000
@@ -24,8 +24,10 @@
24 page elements.24 page elements.
25 */25 */
26 // The button is inserted in the page dynamically:26 // The button is inserted in the page dynamically:
27 // Changed from an <input type=button> to a <button> since
28 // IE8 doesn't handle style.css's input{visibility:inherit} correctly.
27 $('help-close').innerHTML =29 $('help-close').innerHTML =
28 '<input id="help-close-btn" type="button" value="Continue">';30 '<button id="help-close-btn" value="Continue">';
29 forEach(findHelpLinks(), setupHelpTrigger);31 forEach(findHelpLinks(), setupHelpTrigger);
30 initHelpPane();32 initHelpPane();
31}33}

Subscribers

People subscribed via source and target branches

to status/vote changes: