launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

The proposal to merge lp:~sinzui/launchpad/obsolete-js into lp:launchpad has been updated.

Description changed to:

Pre-implementation: no one

While investigating a forbidden error during an ajax request, I was
surprised to see comments by kiko about obsolete code. While the
forbidden error should have been handled gracefully by the ajax request,
there is a separate issue that every page in Lp is serving obsolete
JavaScript functions via base-layout-macros.pt.

All the functions in <metal:page-javascript define-macro="page-javascript"...
need review:
    * VOID_URL is already inlined in it's only callsite, remove it.
    * registerLaunchpadFunction() is only used by codeimport-new.pt; inline
      the function.
    * updateField() is only used by codeimport-new.pt; inline the function.
    * setBetaRedirect() is used by the deprecated edge server; remove it.
    * popup_window() has no callsites; remove it.
    * switchBugBranchFormAndWhiteboard() has no callsites; remove it.
    * switchSpecBranchFormAndSummary() is used by branch-related-bug-specs.pt
      and branch-macros.pt. It could be inlined but it might belong in a
      code/javascript module.

--------------------------------------------------------------------

RULES

    * Delete and move until happiness is achieved.

    ADDENDUM
    * Bug #968310 javascript error editing a branch's related blueprints
      switchSpecBranchFormAndSummary() errors on the branch page because
      the form/markup that it used was removed. The function is 5 years
      old an probably broke in the 2x or 3x redesign.

    DOUBLE ADDENDUM
    * WTF. The switchSpecBranchFormAndSummary() function is not only
      causing scripting errors on the branch page, it was disabled
      on the merge page years ago because it just does not work.
      * delete switchSpecBranchFormAndSummary()

QA

    * Visit https://code.qastaging.launchpad.net/+code-imports/+new
    * Verify that when a repo type is selectected, its url field is
      is enabled and the other are disabled.

    * Visit https://code.qastaging.launchpad.net/~sinzui/gdp/incubation
    * Verify there is not a green 'Edit' text after the blueprint link.
    * Verify there is not a JS error when the edit link is followed.

LINT

    lib/lp/app/templates/base-layout-macros.pt
    lib/lp/code/templates/branch-macros.pt
    lib/lp/code/templates/branch-related-bugs-specs.pt
    lib/lp/code/templates/codeimport-new.pt

TEST

    None. These were untested functions though I do know exactly how I will
    QA the moved functions.

IMPLEMENTATION

Inlined updateWidgets() and registerLaunchpadFunction()'s LPJS setup. I
changed the call to run on DOMREADY and I chose to make it clear that
that the functions are global vars.
    lib/lp/code/templates/codeimport-new.pt

I deleted the uses of switchSpecBranchFormAndSummary() which broke the
branch page (branch-related-bugs-specs) and was disabled on the merge
proposal page (branch-macros). In the former case I chose to keep the
edit link which is what my browser failed over to use. In the later
case, the edit links were never generated, so I choose to just deleted.
    lib/lp/code/templates/branch-macros.pt
    lib/lp/code/templates/branch-related-bugs-specs.pt

I removed all the unused functions. The two remaining functions are used
by LaunchpadForm and Widgets to set the field that has focus and ensure
that keyboard commands are not misinterpreted by <select>.
    lib/lp/app/templates/base-layout-macros.pt

For more details, see:
https://code.launchpad.net/~sinzui/launchpad/obsolete-js/+merge/99978
-- 
https://code.launchpad.net/~sinzui/launchpad/obsolete-js/+merge/99978
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~sinzui/launchpad/obsolete-js into lp:launchpad.