The rest of the code review. A few minor things, but nothing serious. Rock on :) Get this landed, people want to buy you beer :)


> === modified file 'lib/lp/bugs/browser/configure.zcml'
> --- lib/lp/bugs/browser/configure.zcml	2009-12-09 12:13:11 +0000
> +++ lib/lp/bugs/browser/configure.zcml	2009-12-09 12:13:43 +0000
> @@ -95,6 +95,13 @@
>              facet="bugs"
>              permission="launchpad.AnyPerson"/>
>          <browser:page
> +            name="+filebug-inline-form"
> +            for="lp.bugs.interfaces.bugtarget.IBugTarget"
> +            class="lp.bugs.browser.bugtarget.FilebugShowSimilarBugsView"
> +            template="../templates/bugtarget-filebug-inline-form.pt"
> +            facet="bugs"
> +            permission="launchpad.AnyPerson"/>
> +        <browser:page
>              name="+manage-official-tags"
>              for="lp.bugs.interfaces.bugtarget.IOfficialBugTagTargetRestricted"
>              class="lp.bugs.browser.bugtarget.OfficialBugTagsManageView"
>
> === modified file 'lib/lp/bugs/browser/tests/bugtarget-filebug-views.txt'
> --- lib/lp/bugs/browser/tests/bugtarget-filebug-views.txt	2009-11-26 09:18:22 +0000
> +++ lib/lp/bugs/browser/tests/bugtarget-filebug-views.txt	2009-12-09 12:13:43 +0000
> @@ -43,6 +43,25 @@
>      u'Test description.'
>
>
> +== URLs to additional FileBug elements ==
> +
> +FileBugViewBase provides properties that return the URLs of further
> +useful parts of the +filebug process.
> +
> +The inline_filebug_form_url property returns the URL of the inline
> +filebug form so that it may be loaded asynchronously.
> +
> +    >>> print filebug_view.inline_filebug_form_url
> +    http://launchpad.dev/ubuntu/+source/mozilla-firefox/+filebug-inline-form
> +
> +Similarly, the duplicate_search_url property returns the base URL for
> +the duplicate search view, which can be used to load the list of
> +possible duplicates for a bug asynchronously.
> +
> +    >>> print filebug_view.duplicate_search_url
> +    http://launchpad.dev/ubuntu/+source/mozilla-firefox/+filebug-show-similar
> +
> +
>  == Adding extra info to filed bugs ==
>
>  It's possible for bug reporting tools to upload a file with debug
>
> === modified file 'lib/lp/bugs/interfaces/bug.py'
> --- lib/lp/bugs/interfaces/bug.py	2009-11-26 09:18:22 +0000
> +++ lib/lp/bugs/interfaces/bug.py	2009-12-09 12:13:43 +0000
> @@ -26,6 +26,7 @@
>  from zope.interface import Interface, Attribute
>  from zope.schema import (
>      Bool, Bytes, Choice, Datetime, Int, List, Object, Text, TextLine)
> +from zope.schema.vocabulary import SimpleVocabulary
>  from zope.security.interfaces import Unauthorized
>
>  from canonical.launchpad import _
> @@ -789,6 +790,11 @@
>          "A sequence of IBugTaskDeltas, one IBugTaskDelta or None.")
>
>
> +# A simple vocabulary for the subscribe_to_existing_bug form field.
> +SUBSCRIBE_TO_BUG_VOCABULARY = SimpleVocabulary.fromItems(
> +    [('yes', True), ('no', False)])
> +
> +
>  class IBugAddForm(IBug):
>      """Information we need to create a bug"""
>      id = Int(title=_("Bug #"), required=False)
> @@ -842,8 +848,9 @@
>      assignee = PublicPersonChoice(
>          title=_('Assign to'), required=False,
>          vocabulary='ValidAssignee')
> -    subscribe_to_existing_bug = Bool(
> -        title=_(u'Subscribe to this bug'),
> +    subscribe_to_existing_bug = Choice(
> +        title=u'Subscribe to this bug',
> +        vocabulary=SUBSCRIBE_TO_BUG_VOCABULARY,
>          required=True, default=False)
>
>
>
> === added file 'lib/lp/bugs/templates/bugtarget-filebug-inline-form.pt'
> --- lib/lp/bugs/templates/bugtarget-filebug-inline-form.pt	1970-01-01 00:00:00 +0000
> +++ lib/lp/bugs/templates/bugtarget-filebug-inline-form.pt	2009-12-09 12:13:43 +0000
> @@ -0,0 +1,11 @@
> +<tal:root
> +  xmlns:tal="http://xml.zope.org/namespaces/tal"
> +  xmlns:metal="http://xml.zope.org/namespaces/metal"
> +  omit-tag="">
> +
> +  <tal:filebug-form define="launchpad_form_id string:filebug-form">
> +    <metal:display-similar-bugs
> +        use-macro="context/@@+filebug-macros/inline-filebug-form" />
> +  </tal:filebug-form>
> +
> +</tal:root>
>
> === modified file 'lib/lp/bugs/templates/bugtarget-filebug-search.pt'
> --- lib/lp/bugs/templates/bugtarget-filebug-search.pt	2009-11-26 09:18:22 +0000
> +++ lib/lp/bugs/templates/bugtarget-filebug-search.pt	2009-12-09 12:13:43 +0000
> @@ -1,10 +1,26 @@
> -<filebug-search
> +<html
>    xmlns="http://www.w3.org/1999/xhtml"
>    xmlns:tal="http://xml.zope.org/namespaces/tal"
>    xmlns:metal="http://xml.zope.org/namespaces/metal"
>    xmlns:i18n="http://xml.zope.org/namespaces/i18n"
>    metal:use-macro="view/macro:page/main_only">
>
> +  <metal:block fill-slot="head_epilogue">
> +    <script type="text/javascript"
> +            tal:condition="devmode"
> +            tal:content="string:var yui_base='${yui}';"></script>
> +    <script type="text/javascript"
> +            tal:condition="devmode"
> +            tal:define="lp_js string:${icingroot}/build"
> +            tal:attributes="src string:${lp_js}/bugs/filebug-dupefinder.js"></script>

Oh man, I hate this boilerplate everywhere. Not your fault, and this
isn't asking you to do anything. I just wonder if there's a better way
though. I'll email the list if I'm still bothered later this
afternoon.

> +    <script type="text/javascript">
> +        LPS.use(
> +          'base', 'node', 'oop', 'event', 'bugs.dupe_finder', function(Y) {
> +            Y.bugs.setup_dupe_finder();
> +        });
> +    </script>
> +  </metal:block>
> +
>    <div metal:fill-slot="heading">
>      <div tal:condition="view/isPrivate" id="privacy" class="aside private">
>        This report will be private, though you can disclose it later.
> @@ -23,38 +39,91 @@
>        </tal:does-not-use-malone>
>
>        <tal:uses-malone tal:condition="view/contextUsesMalone">
> -        <div class="top-portlet">
> +        <div class="top-portlet"
> +            tal:define="launchpad_form_id string:filebug-search-form">
>            <div metal:use-macro="context/@@launchpad_form/form">
> -            <table class="form" metal:fill-slot="widgets">
> -              <tal:product_widget
> -                  tal:define="widget nocall:view/widgets/product|nothing"
> -                  tal:condition="widget">
> -                <metal:widget metal:use-macro="context/@@launchpad_form/widget_row" />
> -              </tal:product_widget>
> -              <tal:bugtarget
> -                  tal:define="widget nocall:view/widgets/bugtarget|nothing"
> -                  tal:condition="widget">
> -                <metal:widget metal:use-macro="context/@@launchpad_form/widget_row" />
> -              </tal:bugtarget>
> -              <tal:hidden_tags tal:replace="structure view/widgets/tags/hidden" />
> -              <tr>
> -                <td colspan="2">
> -                  Please describe the bug in a few words, for example,
> -                  "weather applet crashes on logout":
> -                </td>
> -              </tr>
> -              <tal:title_widget tal:define="widget nocall:view/widgets/title">
> -                <metal:widget use-macro="context/@@launchpad_form/widget_row" />
> -              </tal:title_widget>
> -            </table>
> -            <div class="actions" metal:fill-slot="buttons">
> -              <input tal:replace="structure view/actions/field.actions.search/render" />
> -            </div>
> -          </div>
> +
> +        <table metal:fill-slot="widgets">
> +
> +          <tal:product_widget
> +              tal:define="widget nocall:view/widgets/product|nothing"
> +              tal:condition="widget">
> +            <metal:widget metal:use-macro="context/@@launchpad_form/widget_row" />
> +          </tal:product_widget>
> +
> +          <tal:bugtarget
> +              tal:define="widget nocall:view/widgets/bugtarget|nothing"
> +              tal:condition="widget">
> +            <metal:widget metal:use-macro="context/@@launchpad_form/widget_row" />
> +          </tal:bugtarget>

It seems odd to have both product and bugtarget widgets, but I see
that it's the same as before, so no worries.

> +
> +          <tal:hidden_tags tal:replace="structure view/widgets/tags/hidden" />
> +
> +          <tr>
> +            <td colspan="2">
> +              <p>
> +                Please describe the bug in a few words, for example, "weather
> +                applet crashes on logout":
> +              </p>

I don't think the <p></p> is needed here (i.e. <td>s can have contain
inline content)... unless it affects layout.

> +            </td>
> +          </tr>
> +          <tal:title_widget tal:define="widget nocall:view/widgets/title">
> +            <tal:comment replace="nothing">
> +              The desire to have more control over the styling of this widget
> +              prevents us from using the widget_row macro here.
> +            </tal:comment>
> +            <tr>
> +              <td tal:define="field_name widget/context/__name__;
> +                              error python:view.getFieldError(field_name);
> +                              error_class python:error and 'error' or None;"

I think you can say this now:

  ('error' if error else None)

(Woohoo!)

> +                  tal:attributes="class error_class"
> +                  style="text-align: left; padding-left: 5em;">
> +                <div>
> +                  <label tal:attributes="for widget/name"
> +                         tal:content="string:${widget/label}:">Label</label>
> +                  <input tal:replace="structure widget" />
> +                </div>
> +                <div class="message" tal:condition="error"
> +                     tal:content="structure error">Error message</div>
> +                <p class="formHelp"
> +                   tal:condition="widget/hint"
> +                   tal:content="widget/hint">Some Help Text
> +                </p>
> +              </td>
> +              <td style="text-align: left;">
> +                <input tal:replace="structure view/actions/field.actions.search/render" />
> +                <span id="spinner" style="text-align: center; display: none;">
> +                  <img src="/@@/spinner" />
> +                </span>
> +              </td>
> +            </tr>
> +          </tal:title_widget>
> +        </table>
> +
> +        <div metal:fill-slot="buttons">
> +          <tal:comment replace="nothing">
> +            We add this to hide the standard action buttons.
> +          </tal:comment>
>          </div>
> -      </tal:uses-malone>
> -
> +      </div>
>      </div>
> -  </div>
> -
> -</filebug-search>
> +    </tal:uses-malone>
> +  </div>
> +
> +  <div id="possible-duplicates" style="text-align: left;">
> +  </div>
> +  <div id="filebug-form-container" style="display: none;">
> +  </div>
> +
> +  <p>
> +    <a id="filebug-form-url"
> +        tal:attributes="href view/inline_filebug_form_url"
> +        style="display: none;"></a>
> +    <a id="duplicate-search-url"
> +        tal:attributes="href view/duplicate_search_url"
> +        style="display: none;"></a>
> +  </p>

Maybe it's simpler to put the display:none on the <p>? I think empty
paragraphs still influence layout.

> +
> +  </div>
> +
> +</html>
>
> === modified file 'lib/lp/bugs/templates/bugtarget-filebug-show-similar.pt'
> --- lib/lp/bugs/templates/bugtarget-filebug-show-similar.pt	2009-11-26 09:18:22 +0000
> +++ lib/lp/bugs/templates/bugtarget-filebug-show-similar.pt	2009-12-09 12:13:43 +0000
> @@ -3,7 +3,12 @@
>    xmlns:metal="http://xml.zope.org/namespaces/metal"
>    omit-tag="">
>
> -  <metal:display-similar-bugs
> -      use-macro="context/@@+filebug-macros/display-similar-bugs" />
> +  <tal:similar-bugs-list condition="view/similar_bugs">
> +    <metal:display-similar-bugs
> +        use-macro="context/@@+filebug-macros/show-similar-bugs-and-filebug-button" />
> +  </tal:similar-bugs-list>
> +  <tal:no-similar-bugs condition="not: view/similar_bugs">
> +    <p>No similar bug reports were found.</p>
> +  </tal:no-similar-bugs>
>
>  </tal:root>
>
> === modified file 'lib/lp/bugs/templates/bugtarget-filebug-submit-bug.pt'
> --- lib/lp/bugs/templates/bugtarget-filebug-submit-bug.pt	2009-12-08 10:57:44 +0000
> +++ lib/lp/bugs/templates/bugtarget-filebug-submit-bug.pt	2009-12-09 12:13:43 +0000
> @@ -1,4 +1,4 @@
> -<filebug-submit
> +<div
>    xmlns="http://www.w3.org/1999/xhtml"
>    xmlns:tal="http://xml.zope.org/namespaces/tal"
>    xmlns:metal="http://xml.zope.org/namespaces/metal"
> @@ -64,4 +64,4 @@
>      </div>
>    </div>
>
> -</filebug-submit>
> +</div>
>
> === modified file 'lib/lp/bugs/templates/bugtarget-macros-filebug.pt'
> --- lib/lp/bugs/templates/bugtarget-macros-filebug.pt	2009-11-26 09:18:22 +0000
> +++ lib/lp/bugs/templates/bugtarget-macros-filebug.pt	2009-12-09 12:13:43 +0000
> @@ -100,7 +100,7 @@
>
>    <tr>
>      <td colspan="2">
> -      <fieldset class="collapsible collapsed">
> +      <fieldset id="filebug-extra-options" class="collapsible collapsed">
>          <legend>Extra options</legend>
>          <table>
>            <tal:status
> @@ -361,8 +361,19 @@
>  </metal:bug_reporting_guidelines>
>
>  <metal:similar-bugs define-macro="display-similar-bugs">
> -  <p id="filebug-query-heading">Is "<strong><span tal:replace="view/search_text">my submitted bug
> -    summary</span></strong>" one of these bugs?</p>
> +  <tal:results-intro-with-summary condition="view/show_summary_in_results">
> +    <p id="filebug-query-heading">
> +      Is "<strong><span tal:replace="view/search_text">my submitted bug summary</span></strong>"
> +      one of these bugs?
> +    </p>
> +  </tal:results-intro-with-summary>
> +  <tal:results-intro-without-summary
> +      condition="not: view/show_summary_in_results">
> +    <p id="filebug-query-heading">
> +      Do any of the following bugs describe the bug you're trying to
> +      report?
> +    </p>
> +  </tal:results-intro-without-summary>
>    <ul id="similar-bugs">
>      <li tal:repeat="bug view/similar_bugs" class="similar-bug">
>        <table tal:define="bugtask python:view.getRelevantBugTask(bug)">
> @@ -461,8 +472,9 @@
>       tal:condition="error_message" tal:content="error_message" class="error message" />
>  </metal:similar-bugs>
>
> -<metal:similar-bugs-and-filebug-form
> -    define-macro="show-similar-bugs-and-filebug-form">
> +<metal:similar-bugs-and-filebug-button
> +    define-macro="show-similar-bugs-and-filebug-button">
> +
>    <tal:similar-bugs condition="view/show_duplicate_list">
>      <metal:similar-bugs
>          use-macro="context/@@+filebug-macros/display-similar-bugs" />
> @@ -477,22 +489,55 @@
>               value="No, I need to report a new bug" />
>      </p>
>    </tal:similar-bugs>
> +</metal:similar-bugs-and-filebug-button>
> +
> +<metal:similar-bugs-and-filebug-form
> +    define-macro="show-similar-bugs-and-filebug-form">
> +
> +  <metal:similar-bugs
> +      use-macro="context/@@+filebug-macros/show-similar-bugs-and-filebug-button" />
> +
>    <tal:submit-new-bug>
> -    <div id="bug_reporting_form">
> -      <metal:form use-macro="context/@@launchpad_form/form">
> -        <a name="form-start" />
> -        <metal:widgets metal:fill-slot="widgets">
> -          <table class="form">
> -            <metal:basic_filebug_widgets
> -                metal:use-macro="context/@@+filebug-macros/basic_filebug_widgets" />
> -          </table>
> -        </metal:widgets>
> -        <div class="actions" metal:fill-slot="buttons">
> -          <input tal:replace="structure view/submit_bug_action/render" />
> -        </div>
> -      </metal:form>
> -    </div>
> +    <metal:basic_filebug_widgets
> +        metal:use-macro="context/@@+filebug-macros/inline-filebug-form" />
>    </tal:submit-new-bug>
>  </metal:similar-bugs-and-filebug-form>
>
> +<metal:inline-filebug-form define-macro="inline-filebug-form">
> +  <div id="bug_reporting_form">
> +    <metal:form use-macro="context/@@launchpad_form/form">
> +      <a name="form-start" />
> +      <metal:widgets metal:fill-slot="widgets">
> +        <table class="form">
> +          <metal:basic_filebug_widgets
> +              metal:use-macro="context/@@+filebug-macros/basic_filebug_widgets" />
> +        </table>
> +      </metal:widgets>
> +      <div class="actions" metal:fill-slot="buttons">
> +        <input tal:replace="structure view/submit_bug_action/render" />
> +      </div>
> +    </metal:form>
> +  </div>
> +</metal:inline-filebug-form>
> +
> +<metal:filebug-form define-macro="simple-filebug-form">
> +  <form action="."
> +        tal:attributes="action view/action_url"
> +        name="launchpadform"
> +        method="post"
> +        enctype="multipart/form-data"
> +        accept-charset="UTF-8"
> +        id="filebug-form">
> +
> +    <table class="form">
> +      <metal:basic_filebug_widgets
> +          metal:use-macro="context/@@+filebug-macros/basic_filebug_widgets" />
> +    </table>
> +    <div class="actions">
> +      <input tal:replace="structure view/submit_bug_action/render" />
> +    </div>
> +
> +  </form>
> +</metal:filebug-form>
> +
>  </tal:root>
>