980903 - Remove onclick from devtools/netmonitor/netmonitor.xul

Status: RESOLVED FIXED

(DevTools :: Netmonitor, defect)

Description

[Mark Goodwin [:mgoodwin]]

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);

Comment 1

[Avik Pal [:avikpal]]

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.

Comment 2

[Avik Pal [:avikpal]]

Attachment: statistics-back-button-removed.patch

Network statistics back button removed along with activity.

Comment 3

[Panos Astithas (he/him) [:past] (please ni?)]

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).

Comment 4

[Victor Porof [:vporof][:vp]]

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?

Comment 5

[Avik Pal [:avikpal]]

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?

Comment 6

[Panos Astithas (he/him) [:past] (please ni?)]

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.

Comment 7

[amanjain110893]

hey I want to work on this bug can someone please assigned it to me ?

I am newbie and I urge for proper guidance

Comment 8

[Panos Astithas (he/him) [:past] (please ni?)]

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.

Comment 9

[Earle Castledine]

Attachment: remove inline click handler for statistics back button

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.

Comment 10

[Earle Castledine]

Attachment: remove inline click handler for statistics back button

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.

Comment 11

[Panos Astithas (he/him) [:past] (please ni?)]

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

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/ae3d07b6778c

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/ae3d07b6778c