Remove declaration of nsPresContext::IsChromeSlow

RESOLVED FIXED in Firefox 52

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: mstange, Assigned: p23keshav, Mentored)

Tracking

({good-first-bug})

Trunk
mozilla52
good-first-bug
Points:
---

Firefox Tracking Flags

(firefox52 fixed)

Details

(Whiteboard: [lang=c++])

Attachments

(2 attachments)

(Reporter)

Description

2 years ago
http://searchfox.org/mozilla-central/rev/021e8e0cee4f446757b8b1ffd312272174d6fc7b/layout/base/nsPresContext.h#1204

I really like the method name, but there is no corresponding implementation of this method and nobody is calling it.

This was missed in bug 969099 when the method was removed.
(Reporter)

Updated

2 years ago
Keywords: good-first-bug
Whiteboard: [lang=c++]
(Assignee)

Comment 1

2 years ago
Hey Markus! I want to contribute to mozilla. Can I take this up for my first bug?
(Reporter)

Comment 2

2 years ago
Sure! Let me know if you need help with anything.
Assignee: nobody → p23keshav
Status: NEW → ASSIGNED
(Assignee)

Comment 3

2 years ago
Thank You!
(Assignee)

Comment 4

2 years ago
Hi! Do I just need to remove the line 1204 in nsPresContext.h?
(Assignee)

Comment 5

2 years ago
I am using hg qnew command to create a patch. There's an error that inotify watchman does not have free space.
(Reporter)

Comment 6

2 years ago
(In reply to p23keshav from comment #4)
> Hi! Do I just need to remove the line 1204 in nsPresContext.h?

Correct! And the empty line below it.

(In reply to p23keshav from comment #5)
> I am using hg qnew command to create a patch. There's an error that inotify
> watchman does not have free space.

I haven't seen that error before. If it's just a warning, maybe you should just ignore it. If it breaks things, you could try turning off the watchman extension by removing the corresponding line from the [extensions] section of your .hgrc.
(Assignee)

Comment 7

2 years ago
Created attachment 8807058 [details] [diff] [review]
bug1314158_removeddeclaration.diff
(Reporter)

Comment 8

2 years ago
Comment on attachment 8807058 [details] [diff] [review]
bug1314158_removeddeclaration.diff

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

Thanks!

The patch looks good and I've marked the patch as reviewed.

Usually, when attaching a patch, you need to request review from somebody. This indicates that you think the patch is ready to go, and automatically notifies the reviewer. See https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch

Now the next step is to get the patch checked in to the tree. In order for that to happen, please upload the patch again, but with an updated commit message that reflects the review ("r=mstange" at the end), and with authorship information. Then you can add the checkin-needed keyword to the bug, and somebody will come and land the patch for you.

So the new patch would have the commit message: "Bug 1314158 - Remove declaration of IsChromeSlow in nsPresContext.h. r=mstange", and your name + email address in the Author metadata. You can use a pseudonym if you don't want your real name to show up.

Usually, when you generate a patch with mercurial, the Author metadata is automatically included. But only if you have told mercurial about your name + email address, by including the information in your .hgrc:

[ui]
username = John Doe <johndoe@mydomain.com>

If you run "mach mercurial-setup", it will automatically add this section for you. See https://developer.mozilla.org/en-US/docs/Mozilla/Mercurial/Installing_Mercurial

Please let me know if anything is unclear.
Attachment #8807058 - Flags: review+
(Assignee)

Comment 9

2 years ago
Created attachment 8807425 [details] [diff] [review]
bug1314158_removeddeclaration.diff

Hi mstange! The thing is when I run mercurial setup,it asks me to press enter to continue and when I do it, it exits from the setup. I edited the username in .hgrc file but the new patch I create still not shows the username. Maybe it's because of the watchman warning I was getting? Also is the extension for watchman in hgrc- "fsmonitor"? 

also can you vouch for my account- https://mozillians.org/en-US/u/p23keshav/

I am also interested in GSOC. Can you give some guidelines?
Attachment #8807425 - Flags: review+
(Assignee)

Updated

2 years ago
Attachment #8807425 - Flags: review+
(Assignee)

Updated

2 years ago
Flags: needinfo?(mstange)

Comment 10

2 years ago
Pushed by mstange@themasta.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/72436161232c
Remove declaration of IsChromeSlow in nsPresContext.h. r=mstange
(Reporter)

Comment 11

2 years ago
Hi Keshav! Sorry for not responding earlier, I was quite busy.

(In reply to p23keshav from comment #9)
> Hi mstange! The thing is when I run mercurial setup,it asks me to press
> enter to continue and when I do it, it exits from the setup.

Hmm, that's very strange. Which operating system are you using?

> I edited the
> username in .hgrc file but the new patch I create still not shows the
> username.

The patch you attached contains the line I was looking for: "# User Keshav Prasad <p23keshav@gmail.com>"
Did you add this manually? If not, then things are working correctly now.

> also can you vouch for my account- https://mozillians.org/en-US/u/p23keshav/

I'd prefer to see you write at least one or two more patches before I'm comfortable vouching.

> I am also interested in GSOC. Can you give some guidelines?

I'd like to direct you to Mike Hoye <mhoye@mozilla.com> for this question. If you send him an email I'm sure he'll give you some information on it.
Flags: needinfo?(mstange)

Comment 12

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/72436161232c
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox52: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.