Minor JS cleanup for the "edit multiple bugs" page

RESOLVED FIXED in Bugzilla 3.6

Status

()

Bugzilla
User Interface
--
minor
RESOLVED FIXED
10 years ago
9 years ago

People

(Reporter: Christian Höltje, Assigned: Christian Höltje)

Tracking

3.0.4
Bugzilla 3.6
Bug Flags:
approval +

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

10 years ago
Created attachment 333937 [details] [diff] [review]
Minor cleanup of the edit-multiple template

Attached is a minor patch to clean up the JS in the edit multiple bugs page.

I did it while tracking down a different problem.

This patch generates less namespace pollution, won't accidentally check boxes that aren't part of the list (if we ever add any) and uses the current values on the page.

Comment 1

10 years ago
  See our development process:

  http://wiki.mozilla.org/Bugzilla:Developers
Keywords: checkin-needed
OS: Mac OS X → All
Hardware: PC → All
(Assignee)

Comment 2

10 years ago
(In reply to comment #1)
>   See our development process:
> 
>   http://wiki.mozilla.org/Bugzilla:Developers

Thank for pointing me at that.  The wiki page doesn't have a reviewer for JavaScript, so who would be the requestee?

Ciao!

Comment 3

10 years ago
(In reply to comment #2)
> Thank for pointing me at that.  The wiki page doesn't have a reviewer for
> JavaScript, so who would be the requestee?

  Try guy.pyrzak (gmail)
(Assignee)

Comment 4

10 years ago
Comment on attachment 333937 [details] [diff] [review]
Minor cleanup of the edit-multiple template

Please review this minor change to the JS.  It's mainly a cleanup and it makes it more robust by explicitly checking for checkboxes and avoiding any that don't match the id ~= m/^id_.*/ pattern.

Ciao!
Attachment #333937 - Flags: review?(guy.pyrzak)

Comment 5

10 years ago
Comment on attachment 333937 [details] [diff] [review]
Minor cleanup of the edit-multiple template

So there are some spacing issues, (you should have spaces around all the operators in the for statement and no spaces before the semicolons) however, those were there before and could be fixed on submit.
Attachment #333937 - Flags: review?(mkanat)
Attachment #333937 - Flags: review?(guy.pyrzak)
Attachment #333937 - Flags: review+
(Assignee)

Comment 6

10 years ago
Just for future reference, it should look like this?
for (i = 0; i < foo; i++) {...

Comment 7

10 years ago
(In reply to comment #6)
> Just for future reference, it should look like this?
> for (i = 0; i < foo; i++) {...

  Yep. And if you could submit a new patch that looks like that, it'd be nice.

Comment 8

10 years ago
Please don't write [patch] in the bug summary.
Assignee: ui → docwhat+mozillabugs
Severity: normal → minor
Summary: [patch] minor JS cleanup for edit multiple bugs page. → Minor JS cleanup for the "edit multiple bugs" page
(Assignee)

Comment 9

10 years ago
Created attachment 334625 [details] [diff] [review]
Minor cleanup of the edit-multiple template

(In reply to comment #7)
>   Yep. And if you could submit a new patch that looks like that, it'd be nice.

Sure!
Attachment #333937 - Attachment is obsolete: true
Attachment #333937 - Flags: review?(mkanat)

Comment 10

10 years ago
i like this, but i think it's appropriate for 3.4, not 3.2
Target Milestone: --- → Bugzilla 3.4

Comment 11

10 years ago
pyrzak, mkanat: is this patch ready for checkin?
Flags: approval?

Updated

10 years ago
Attachment #334625 - Flags: review+

Comment 12

10 years ago
Comment on attachment 334625 [details] [diff] [review]
Minor cleanup of the edit-multiple template

Yes, although I don't see why you moved the definition of i out of the loop.

Also, { should be aligned with "if" there. (Or that should just be one line)

Updated

10 years ago
Flags: approval? → approval+

Comment 13

10 years ago
mkanat, I will let you commit it so that you can do your fixes on checkin.

Updated

9 years ago
Status: NEW → ASSIGNED
Target Milestone: Bugzilla 3.4 → Bugzilla 3.6

Comment 14

9 years ago
Sorry it took so long to check this in! :-)

Checking in template/en/default/list/edit-multiple.html.tmpl;
/cvsroot/mozilla/webtools/bugzilla/template/en/default/list/edit-multiple.html.tmpl,v  <--  edit-multiple.html.tmpl
new revision: 1.58; previous revision: 1.57
done

Oh, oops! I accidentally credited myself for the patch, as a reflex because I was checking it in. I'm really sorry. The bug here shows that it's attributed to Christian, though, and it was definitely his patch, not mine. :-)
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.