launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #07806
[Merge] lp:~sinzui/launchpad/halt-when-done into lp:launchpad
Curtis Hovey has proposed merging lp:~sinzui/launchpad/halt-when-done into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #355164 in Launchpad itself: "ajax duplicate marker not working on internet explorer 7"
https://bugs.launchpad.net/launchpad/+bug/355164
Bug #992656 in Launchpad itself: "HTML Parsing Error: Unable to modify the parent container element before the child element is closed"
https://bugs.launchpad.net/launchpad/+bug/992656
Bug #998229 in Launchpad itself: "overylay fields are visible in msie when the overlay is hidden"
https://bugs.launchpad.net/launchpad/+bug/998229
For more details, see:
https://code.launchpad.net/~sinzui/launchpad/halt-when-done/+merge/105542
Pre-implementation: wgrant
I started this branch to fix ChoiceSource, but I found I first had to
fix the load errors caused by broken scripts. I did not fix my bug,
but I have made Lp usable to MSIE users.
--------------------------------------------------------------------
RULES
* View the source of IE to see where rendering stopped to locate
the part of the template to provides the bad script block
* Move the script block or add a DOMReady event listener to ensure
the block is not mutating the parent element while the DOM is
being created.
ADDENDUM
* Oh, the overlays do work. They are not broken by JS, but they are
missing the right CSS. Use display: none instead of visibility: hidden
for forms because IE only handles it correctly in IE9.
* Enable each overlay with an Y.UA.ie guard and verify the fields
are not shown on load, that you can display and submit the overlay.
QA
Page loads without HTML Parsing Error in MSIE.
* Visit https://blueprints.qastaging.launchpad.net/gdp/+spec/gdplaunchpad
* Verify the page loads and that the blueprint description can
be edited.
* Visit https://code.qastaging.launchpad.net/~sinzui/+recipe/pocket-lint-daily
* Verify the page loads and that the recipe description can
be edited.
* Visit https://bugs.qastaging.launchpad.net/gdp/+bug/420173
* Verify the page loads and that the blueprint description can
be edited.
* Verify the bug tag editor works.
* Verify the comment editor works.
Overlays work in MSIE
* Visit https://code.qastaging.launchpad.net/~sinzui/+recipe/pocket-lint-daily
* Verify the target series can be edited.
* Visit https://qastaging.launchpad.net/gdp/+configure-bugtracker
* Verify that an external bug tracker can be registered.
* Visit https://bugs.qastaging.launchpad.net/gdp/+bug/936705
* Verify the duplicate bug overlay works.
* Verify the subscribe overlay works.
* Verify you can link to a related branch.
* Visit the branch
* Verify you can subscribe to the branch
* Propose a merge.
* Visit the bug again.
* Verify you can see the inline diff.
* Visit https://qastaging.launchpad.net/gdp/incubation
* Verify you can create a milestone.
LINT
lib/lp/app/javascript/multicheckbox.js
lib/lp/app/javascript/formoverlay/assets/formoverlay-core.css
lib/lp/app/javascript/overlay/assets/pretty-overlay-core.css
lib/lp/app/templates/inline-multicheckbox-widget.pt
lib/lp/app/templates/text-area-editor.pt
lib/lp/app/widgets/templates/bugtracker-picker.pt
lib/lp/bugs/javascript/bugtask_index.js
lib/lp/bugs/javascript/bugtracker_overlay.js
lib/lp/bugs/templates/bugcomment-macros.pt
lib/lp/bugs/templates/bugtask-index.pt
lib/lp/bugs/templates/bugtasks-and-nominations-portal.pt
lib/lp/code/javascript/branch.subscription.js
lib/lp/code/javascript/branchmergeproposal.diff.js
lib/lp/code/templates/branch-related-bugs-specs.pt
lib/lp/code/templates/sourcepackagerecipe-index.pt
lib/lp/registry/javascript/milestoneoverlay.js
lib/lp/registry/templates/productrelease-add-from-series.pt
^ Lint does not like some of the older js. I can fix the spacing and
semi-colon issues before I land. There are indentation inconsistencies
in the templates that I can fix too.
TEST
./bin/test -vvc --layer=YUITest lp
./bin/test -vvc -t bugtask-management lp.bugs.tests.test_doc
IMPLEMENTATION
Realised the broken overlay was caused by naive use the visibility and
box shadow CSS properties. Most designers will use The css display property
which is properly supported (and invented) by msie. I added two shadows
to using ms' filter property. The older syntax for IE4-7 is not supported
by our build tools...they choke on the non-standard syntax...c'est la vie.
lib/lp/app/javascript/formoverlay/assets/formoverlay-core.css
lib/lp/app/javascript/overlay/assets/pretty-overlay-core.css
Enabled the pretty overlay, form overlay, and overlay widgets for IE.
lib/lp/app/javascript/multicheckbox.js
lib/lp/app/templates/inline-multicheckbox-widget.pt
lib/lp/app/widgets/templates/bugtracker-picker.pt
lib/lp/bugs/javascript/bugtask_index.js
lib/lp/bugs/javascript/bugtracker_overlay.js
lib/lp/code/javascript/branch.subscription.js
lib/lp/code/javascript/branchmergeproposal.diff.js
lib/lp/code/templates/branch-related-bugs-specs.pt
lib/lp/code/templates/sourcepackagerecipe-index.pt
lib/lp/registry/javascript/milestoneoverlay.js
lib/lp/registry/templates/productrelease-add-from-series.pt
Move script that mutate the parent element out of the parent. Set some
to on on DOMReady to ensure the nodes that must be changed are available.
This was a long painful task to prove I could load the bug page. Fixing the
text-area-editor also fixed the known bug for recipes and blueprints.
lib/lp/app/templates/text-area-editor.pt
lib/lp/bugs/templates/bugcomment-macros.pt
lib/lp/bugs/templates/bugtask-index.pt
lib/lp/bugs/templates/bugtasks-and-nominations-portal.pt
--
https://code.launchpad.net/~sinzui/launchpad/halt-when-done/+merge/105542
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/halt-when-done into lp:launchpad.
=== modified file 'lib/lp/app/javascript/formoverlay/assets/formoverlay-core.css'
--- lib/lp/app/javascript/formoverlay/assets/formoverlay-core.css 2011-08-19 14:59:06 +0000
+++ lib/lp/app/javascript/formoverlay/assets/formoverlay-core.css 2012-05-12 00:20:25 +0000
@@ -2,6 +2,11 @@
visibility: hidden;
}
+.yui3-lazr-formoverlay-hidden input {
+ /* msie 8 and below do not support visibility:hidden for <input> */
+ display: none;
+}
+
.yui3-lazr-formoverlay-form th, .yui3-lazr-formoverlay-form td {
/* The same as the Launchpad style, so the example represents
* how it will look.
=== modified file 'lib/lp/app/javascript/multicheckbox.js'
--- lib/lp/app/javascript/multicheckbox.js 2012-01-11 13:10:45 +0000
+++ lib/lp/app/javascript/multicheckbox.js 2012-05-12 00:20:25 +0000
@@ -23,11 +23,6 @@
items, help_text, resource_uri, attribute_name, attribute_type,
content_box_id, config, client) {
-
- if (Y.UA.ie) {
- return;
- }
-
// We may have been passed a mock client for testing but if not, create
// a proper one.
if (client === undefined)
@@ -98,10 +93,6 @@
* a single string argument.
*/
namespace.create = function (attribute_name, items, help_text, config) {
- if (Y.UA.ie) {
- return;
- }
-
if (config !== undefined) {
var header = 'Choose an item.';
if (config.header !== undefined) {
=== modified file 'lib/lp/app/javascript/overlay/assets/pretty-overlay-core.css'
--- lib/lp/app/javascript/overlay/assets/pretty-overlay-core.css 2012-03-10 15:33:33 +0000
+++ lib/lp/app/javascript/overlay/assets/pretty-overlay-core.css 2012-05-12 00:20:25 +0000
@@ -22,6 +22,7 @@
text-align: left;
border-radius: 5px;
-webkit-box-shadow: 0px 0px 20px 10px #aaa;
+ -ms-filter: "progid:DXImageTransform.Microsoft.Shadow(Strength=15, Direction=135, Color='#aaaaaa'), progid:DXImageTransform.Microsoft.Shadow(Strength=15, Direction=315, Color='#aaaaaa')";
box-shadow: 0px 0px 20px 10px #aaa;
}
=== modified file 'lib/lp/app/templates/inline-multicheckbox-widget.pt'
--- lib/lp/app/templates/inline-multicheckbox-widget.pt 2012-04-26 15:48:09 +0000
+++ lib/lp/app/templates/inline-multicheckbox-widget.pt 2012-05-12 00:20:25 +0000
@@ -32,10 +32,6 @@
<script tal:condition="view/can_write"
tal:content="string:
LPJS.use('lp.app.multicheckbox', function(Y) {
- if (Y.UA.ie) {
- return;
- }
-
Y.on('load', function(e) {
Y.lp.app.multicheckbox.addMultiCheckboxPatcher(
${view/json_items},
=== modified file 'lib/lp/app/templates/text-area-editor.pt'
--- lib/lp/app/templates/text-area-editor.pt 2012-03-21 01:27:04 +0000
+++ lib/lp/app/templates/text-area-editor.pt 2012-05-12 00:20:25 +0000
@@ -1,6 +1,6 @@
<div
- xmlns:tal="http://xml.zope.org/namespaces/tal";
- tal:attributes="id view/content_box_id;
+ xmlns:tal="http://xml.zope.org/namespaces/tal";>
+ <div tal:attributes="id view/content_box_id;
class view/tag_class">
<div class="clearfix">
<div class="edit-controls" tal:condition="view/can_write">
@@ -14,6 +14,7 @@
</div>
<div class="yui3-editable_text-text"
tal:content="structure view/value">some text</div>
+ </div>
<script tal:condition="view/can_write"
tal:content="structure string:
LPJS.use('lazr.editor', 'lp.client.plugins', function (Y) {
=== modified file 'lib/lp/app/widgets/templates/bugtracker-picker.pt'
--- lib/lp/app/widgets/templates/bugtracker-picker.pt 2012-02-01 15:31:32 +0000
+++ lib/lp/app/widgets/templates/bugtracker-picker.pt 2012-05-12 00:20:25 +0000
@@ -6,9 +6,6 @@
<metal:form-picker use-macro="context/@@form-picker-macros/form-picker"/>
<script tal:content="structure string:
LPJS.use('lp.bugs.bugtracker_overlay', function(Y) {
- if (Y.UA.ie) {
- return;
- }
Y.on('domready', function () {
// After the success handler finishes, it calls the
// next_step function.
=== modified file 'lib/lp/bugs/javascript/bugtask_index.js'
--- lib/lp/bugs/javascript/bugtask_index.js 2012-04-26 19:11:37 +0000
+++ lib/lp/bugs/javascript/bugtask_index.js 2012-05-12 00:20:25 +0000
@@ -46,14 +46,7 @@
* Check the page for links related to overlay forms and request the HTML
* for these forms.
*/
- Y.on('load', function() {
- /* Skip all this in IE for now. The dupe form's textbox is shown
- * even when the form is hidden, and there's possibly other
- * issues.
- */
- if (Y.UA.ie) {
- return;
- }
+ Y.on('domready', function() {
// If the user is not logged in, then we need to defer to the
// default behaviour.
if (LP.links.me === undefined) {
=== modified file 'lib/lp/bugs/javascript/bugtracker_overlay.js'
--- lib/lp/bugs/javascript/bugtracker_overlay.js 2011-08-09 14:18:02 +0000
+++ lib/lp/bugs/javascript/bugtracker_overlay.js 2012-05-12 00:20:25 +0000
@@ -107,9 +107,6 @@
*/
namespace.attach_widget = function(config) {
Y.log('lp.bugs.bugtracker_overlay.attach_widget()');
- if (Y.UA.ie) {
- return;
- }
if (config === undefined) {
throw new Error(
"Missing attach_widget config for bugtracker_overlay.");
=== modified file 'lib/lp/bugs/templates/bugcomment-macros.pt'
--- lib/lp/bugs/templates/bugcomment-macros.pt 2012-02-01 15:31:32 +0000
+++ lib/lp/bugs/templates/bugcomment-macros.pt 2012-05-12 00:20:25 +0000
@@ -85,6 +85,7 @@
xmlns:tal="http://xml.zope.org/namespaces/tal";
xmlns:metal="http://xml.zope.org/namespaces/metal";
metal:define-macro="comment-form">
+ <div>
<div tal:define="comment_form nocall:context/@@+addcomment-form;
dummy comment_form/initialize" id="add-comment-form">
<div tal:condition="context/bug/duplicateof"
@@ -115,15 +116,18 @@
tal:content="structure
comment_form/actions/field.actions.save/render" />
</form>
- <script type="text/javascript">
- LPJS.use('lp.app.comment', function(Y) {
- var comment = new Y.lp.app.comment.Comment();
- comment.render();
- });
- </script>
</div>
<tal:attachment-link
define="add_attachment_link context_menu/addcomment"
replace="structure add_attachment_link/render" />
</div>
+ <script type="text/javascript">
+ LPJS.use('lp.app.comment', function(Y) {
+ Y.on('domready', function(){
+ var comment = new Y.lp.app.comment.Comment();
+ comment.render();
+ }, window);
+ });
+ </script>
+</div>
</tal:root>
=== modified file 'lib/lp/bugs/templates/bugtask-index.pt'
--- lib/lp/bugs/templates/bugtask-index.pt 2012-03-10 15:08:09 +0000
+++ lib/lp/bugs/templates/bugtask-index.pt 2012-05-12 00:20:25 +0000
@@ -14,11 +14,11 @@
'lp.bugs.subscribers',
'lp.code.branchmergeproposal.diff', 'lp.comments.hide',
function(Y) {
- Y.lp.bugs.bugtask_index.setup_bugtask_index();
- Y.on('load', function(e) {
+ Y.on('domready', function(e) {
Y.lp.code.branchmergeproposal.diff.connect_diff_links();
}, window);
Y.on('domready', function() {
+ Y.lp.bugs.bugtask_index.setup_bugtask_index();
Y.lp.bugs.bugtask_index.setup_bugtask_table();
LP.cache.comment_context = LP.cache.bug;
Y.lp.comments.hide.setup_hide_controls();
@@ -124,15 +124,15 @@
</div>
<div tal:replace="structure context/bug/@@+bugtasks-and-nominations-portal" />
+
<div id="maincontentsub">
- <div><!-- id="nonportlets"> -->
<div class="top-portlet">
<div itemprop="mainContentOfPage" class="report">
<tal:description
define="global description context/bug/description/fmt:obfuscate-email/fmt:text-to-html" />
- <tal:widget replace="structure view/bug_description_html"/>
+ <tal:widget tal:replace="structure view/bug_description_html"/>
<div style="margin:-10px 0 20px 5px" class="clearfix">
<span tal:condition="view/wasDescriptionModified"
@@ -157,28 +157,6 @@
<input type="text" id="tag-input" class="hidden" style="width: 35em;" />
<img src="/@@/spinner" id="tags-edit-spinner" class="hidden"/>
<a href="+edit" title="Edit tags" id="edit-tags-trigger" class="sprite edit"></a>
- <script type="text/javascript">
- LPJS.use('event', 'node', 'lp.bugs.bug_tags_entry', function(Y) {
- // XXX intellectronica 2009-04-16 bug #362309:
- // The load event fires very late on bug pages that take a
- // long time to render, but we prefer to use it since the
- // domready event isn't reliable in YUI3 yet. To make sure
- // that user doesn't click the trigger and navigates to the
- // non-JS form, we first disable the link. This is awful and
- // must be fixed as soon as YUI3 is fixed.
- if (LP.links['me'] !== undefined) {
- Y.one('#edit-tags-trigger').on('click', function(e) {
- e.halt();
- });
- }
- Y.on('load',
- function(e) {
- Y.lp.bugs.bug_tags_entry.setup_tag_entry(
- available_official_tags);
- },
- window);
- });
- </script>
<button class="lazr-pos lazr-btn yui-ieditor-submit_button hidden"
id="edit-tags-ok"
@@ -197,7 +175,22 @@
Add tags
</a>
<a href="/+help-bugs/tag-help.html" target="help" class="sprite maybe">
- <span class="invisible-link">Tag help</span></a>
+ <span class="invisible-link">Tag help</span></a>
+ <script type="text/javascript">
+ LPJS.use('event', 'node', 'lp.bugs.bug_tags_entry', function(Y) {
+ if (LP.links['me'] !== undefined) {
+ Y.one('#edit-tags-trigger').on('click', function(e) {
+ e.halt();
+ });
+ }
+ Y.on('domready',
+ function(e) {
+ Y.lp.bugs.bug_tags_entry.setup_tag_entry(
+ available_official_tags);
+ },
+ window);
+ });
+ </script>
</div>
<div class="clearfix"></div>
@@ -235,6 +228,9 @@
<div class="clearfix"></div>
</div> <!-- branches and CVEs -->
+ </div>
+
+ <div>
<tal:comments>
<tal:comment repeat="comment_or_activity view/activity_and_comments">
<tal:is-comment
@@ -310,7 +306,6 @@
</tal:comments>
</div><!-- class="top-portlet" -->
- </div><!-- id="nonportlets"-->
</div><!--- id="maincontentsub"-->
<div>
<div id="duplicate-form-container"></div>
=== modified file 'lib/lp/bugs/templates/bugtasks-and-nominations-portal.pt'
--- lib/lp/bugs/templates/bugtasks-and-nominations-portal.pt 2012-04-03 06:14:09 +0000
+++ lib/lp/bugs/templates/bugtasks-and-nominations-portal.pt 2012-05-12 00:20:25 +0000
@@ -36,9 +36,10 @@
<img class="editicon" src="/@@/edit" alt="Edit" />
</a>
</span>
+ </span>
<script type="text/javascript" tal:content="string:
LPJS.use('event', 'lp.bugs.bugtask_index', function(Y) {
- Y.on('load', function(e) {
+ Y.on('domready', function(e) {
Y.lp.bugs.bugtask_index.setup_me_too(
${view/current_user_affected_js_status},
${view/other_users_affected_count});
@@ -46,7 +47,6 @@
});
">
</script>
- </span>
</div>
</tal:editable>
<tal:not-editable
=== modified file 'lib/lp/code/javascript/branch.subscription.js'
--- lib/lp/code/javascript/branch.subscription.js 2011-08-09 14:18:02 +0000
+++ lib/lp/code/javascript/branch.subscription.js 2012-05-12 00:20:25 +0000
@@ -197,10 +197,6 @@
bindUI: function() {
var form_overlay = this._form_overlay;
this.get("self_subscribe").on('click', function(e) {
- // IE tables don't support innerHTML after render.
- if (Y.UA.ie) {
- return;
- }
e.halt();
form_overlay.show();
});
=== modified file 'lib/lp/code/javascript/branchmergeproposal.diff.js'
--- lib/lp/code/javascript/branchmergeproposal.diff.js 2011-11-16 14:10:32 +0000
+++ lib/lp/code/javascript/branchmergeproposal.diff.js 2012-05-12 00:20:25 +0000
@@ -135,11 +135,6 @@
* Connect the diff links to their pretty overlay function.
*/
namespace.connect_diff_links = function() {
- // IE doesn't like pretty overlays.
- if (Y.UA.ie) {
- return;
- }
-
// Setup the LP client.
var lp_client = new Y.lp.client.Launchpad();
=== modified file 'lib/lp/code/templates/branch-related-bugs-specs.pt'
--- lib/lp/code/templates/branch-related-bugs-specs.pt 2012-03-29 15:27:48 +0000
+++ lib/lp/code/templates/branch-related-bugs-specs.pt 2012-05-12 00:20:25 +0000
@@ -72,11 +72,6 @@
<!--
LPJS.use('io-base', 'lp.code.branch.bugspeclinks', function(Y) {
-
- if(Y.UA.ie) {
- return;
- }
-
Y.on('domready', function() {
var logged_in = LP.links['me'] !== undefined;
=== modified file 'lib/lp/code/templates/sourcepackagerecipe-index.pt'
--- lib/lp/code/templates/sourcepackagerecipe-index.pt 2012-04-27 23:34:27 +0000
+++ lib/lp/code/templates/sourcepackagerecipe-index.pt 2012-05-12 00:20:25 +0000
@@ -155,10 +155,6 @@
Y.on('lp:context:base_branch_link:changed', function(e) {
Y.lp.ui.update_field('#base-branch dd', e.new_value_html);
});
-
- if(Y.UA.ie) {
- return;
- }
Y.on('load', function() {
Y.lp.code.requestbuild_overlay.hookUpDailyBuildsSchedule();
}, window);
=== modified file 'lib/lp/registry/javascript/milestoneoverlay.js'
--- lib/lp/registry/javascript/milestoneoverlay.js 2012-03-21 01:24:28 +0000
+++ lib/lp/registry/javascript/milestoneoverlay.js 2012-05-12 00:20:25 +0000
@@ -191,12 +191,6 @@
* the milestone is created.
*/
module.attach_widget = function(config) {
- /* In IE8 the text widgets stay around after the form is closed,
- * so disable there for now.
- */
- if (Y.UA.ie) {
- return;
- }
configure(config);
config.activate_node.on('click', show_milestone_form);
};
=== modified file 'lib/lp/registry/templates/productrelease-add-from-series.pt'
--- lib/lp/registry/templates/productrelease-add-from-series.pt 2012-02-01 15:31:32 +0000
+++ lib/lp/registry/templates/productrelease-add-from-series.pt 2012-05-12 00:20:25 +0000
@@ -56,9 +56,7 @@
next_step: add_milestone_to_menu,
activate_node: create_milestone_link
};
- if (!Y.UA.ie) {
- Y.lp.registry.milestoneoverlay.attach_widget(config);
- }
+ Y.lp.registry.milestoneoverlay.attach_widget(config);
});
});
-->
Follow ups
