Closed Bug 980903 Opened 6 years ago Closed 6 years ago

Remove onclick from devtools/netmonitor/netmonitor.xul

Categories

(DevTools :: Netmonitor, defect)

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 31

People

(Reporter: mgoodwin, Assigned: mrspeaker)

References

(Blocks 1 open bug)

Details

(Whiteboard: [mentor=past][lang=html][good first bug][lang=js])

Attachments

(1 file, 2 obsolete files)

I notice, following the inline script cleanup in bug 961085, the network-statistics-back-button still has an onclick attribute (netmonitor.xul line 516-ish);
Hi Mark,

I would like to work on this bug as my first such for mozilla-devtools. Please assign this bug to me if possible.

I have already submitted a patch for this. Please let me know anything else has to be done.
Network statistics back button removed along with activity.
I've assigned the bug to you, but I don't think the intent here is to remove the entire button, just the onclick attribute (and replace it with a dynamically-added one).
Assignee: nobody → avikpal.me
Status: NEW → ASSIGNED
Comment on attachment 8388192 [details] [diff] [review]
statistics-back-button-removed.patch

Review of attachment 8388192 [details] [diff] [review]:
-----------------------------------------------------------------

This is not about removing the button completely.
Brian, care to guide Avik here?
Attachment #8388192 - Flags: review?(vporof) → review-
Thanks for pointing this out, I made a terrible mistake of not understanding it properly. Can you please guide me regarding what can be the dynamic attributes and what are the cases in which they they will take those values?
Brian will be back next week, so until then I would suggest reading this for context:

https://developer.mozilla.org/docs/Web/Guide/Events

The solution here is to not rely on attributes on elements, but on adding event listeners in JS code. You can look into the bugs blocking the dependent bug for inspiration, like bug 961085.
Assignee: avikpal.me → nobody
hey I want to work on this bug can someone please assigned it to me ?

I am newbie and I urge for proper guidance
Hey amanjain, I've assigned the bug to you at your request. If this is your first bug, take a look at:

https://wiki.mozilla.org/DevTools/Hacking

For this particular bug, take a look at comment 6.
Assignee: nobody → amanjain110893
Whiteboard: [mentor=bgrins][lang=html][good first bug][lang=js] → [mentor=past][lang=html][good first bug][lang=js]
I didn't realise this was already assigned - but it was a month ago, so maybe my patch is useful: it removes the inline click handler and adds/removes an event handler in the network view statistics.
Attachment #8408485 - Flags: review?(past)
I asked the people on the #introduction irc channel to review the format of my patch, and they spotted some errors (I was using git patch format). Hopefully fixed now.
Attachment #8408485 - Attachment is obsolete: true
Attachment #8408485 - Flags: review?(past)
Attachment #8409759 - Flags: review?(past)
Attachment #8409759 - Attachment description: Fix the HG patch header → remove inline click handler for statistics back button
Comment on attachment 8409759 [details] [diff] [review]
remove inline click handler for statistics back button

Review of attachment 8409759 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks for the patch! I happened to notice that the patch author field contains a pseudonym, are you sure this is how you want it to land? If so, go ahead and set the checkin-needed keyword in the bug and someone will land it for you. Otherwise attach an updated patch and then flag the bug with checkin-needed.

Looking for another bug to squash? Here is a list of bugs with similar level of difficulty:

http://www.joshmatthews.net/bugsahoy/?devtools=1&simple=1
Attachment #8409759 - Flags: review?(past) → review+
Assignee: amanjain110893 → mrspeaker
Attachment #8388192 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/integration/fx-team/rev/ae3d07b6778c
Keywords: checkin-needed
Whiteboard: [mentor=past][lang=html][good first bug][lang=js] → [mentor=past][lang=html][good first bug][lang=js][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/ae3d07b6778c
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [mentor=past][lang=html][good first bug][lang=js][fixed-in-fx-team] → [mentor=past][lang=html][good first bug][lang=js]
Target Milestone: --- → Firefox 31
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.