Closed
Bug 974097
Opened 11 years ago
Closed 11 years ago
The AppendChild operation in LayersMessages.ipdlh should really be called PrependChild
Categories
(Core :: Graphics: Layers, defect)
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: kats, Assigned: chris.lewis2)
Details
(Whiteboard: [good first bug][lang=c++,ipdl][mentor=kats])
Attachments
(1 file, 1 obsolete file)
4.20 KB,
patch
|
gal
:
review+
|
Details | Diff | Splinter Review |
The implementation in LayerTransactionParent.cpp implements the "AppendChild" op by calling InsertAfter(aChild, null). If you look at the ContainerLayer::InsertAfter implementation in Layers.cpp this is really a prepend operation rather than an append operation - the new child becomes the new first child of the parent.
Assignee | ||
Comment 1•11 years ago
|
||
Can I be assigned to work on this?
Updated•11 years ago
|
Assignee: nobody → chris.lewis2
Comment 2•11 years ago
|
||
Absolutely.
Reporter | ||
Comment 3•11 years ago
|
||
Hi Chris, You can definitely work on this bug. I don't know if you've contributed patches to Mozilla before but if you haven't the first step is to get a build of Firefox up and running. There are instructions on how to to do this at [1]. For this bug a desktop build should be fine. The changes you need to make are pretty straightforward renames of some functions and variables - basically replacing all instances of "AppendChild" in the gfx/layers/ipc/ folder to be "PrependChild" instead. Once you've done that and verified that everything still compiles/works, you can submit a patch for review (see the process at [2]). Please feel free to ask questions either on this bug or via email, or join us on IRC (irc.mozilla.org server, #gfx channel) and ask us there. [1] https://developer.mozilla.org/en/docs/Developer_Guide/Build_Instructions [2] https://developer.mozilla.org/en-US/docs/Developer_Guide/How_to_Submit_a_Patch
Assignee | ||
Comment 4•11 years ago
|
||
Thanks for the assistance, this would be my first patch.
Comment 5•11 years ago
|
||
Can I work on this too?
Assignee | ||
Comment 6•11 years ago
|
||
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #3) > Hi Chris, > > You can definitely work on this bug. I don't know if you've contributed > patches to Mozilla before but if you haven't the first step is to get a > build of Firefox up and running. There are instructions on how to to do this > at [1]. For this bug a desktop build should be fine. > > The changes you need to make are pretty straightforward renames of some > functions and variables - basically replacing all instances of "AppendChild" > in the gfx/layers/ipc/ folder to be "PrependChild" instead. Once you've done > that and verified that everything still compiles/works, you can submit a > patch for review (see the process at [2]). > > Please feel free to ask questions either on this bug or via email, or join > us on IRC (irc.mozilla.org server, #gfx channel) and ask us there. > > [1] https://developer.mozilla.org/en/docs/Developer_Guide/Build_Instructions > [2] > https://developer.mozilla.org/en-US/docs/Developer_Guide/ > How_to_Submit_a_Patch I've created a patch that compiles but I'm not sure what automated test suite to use (re: process at [2]).
Assignee | ||
Comment 7•11 years ago
|
||
Patch compiles but not sure which test suite is appropriate.
Comment 8•11 years ago
|
||
Since this is just a renaming we won't need a test here. No functionality changed.
Comment 9•11 years ago
|
||
Comment on attachment 8380163 [details] [diff] [review] Patch v1 Review of attachment 8380163 [details] [diff] [review]: ----------------------------------------------------------------- If you attach a new patch with the fixes and set checkin-needed in the whiteboard someone will check it in for you. ::: gfx/layers/ipc/LayersMessages.ipdlh @@ +246,5 @@ > > // Monkey with the tree structure > struct OpSetRoot { PLayer root; }; > struct OpInsertAfter { PLayer container; PLayer childLayer; PLayer after; }; > +struct OpPrependChild { PLayer container; PLayer childLayer; }; Please fix the indentation here. ::: gfx/layers/ipc/ShadowLayers.cpp @@ +280,1 @@ > nullptr, Shadow(aChild))); Please fix the indentation here as well.
Attachment #8380163 -
Flags: review+
Assignee | ||
Comment 10•11 years ago
|
||
Thanks for your help Andreas. I've fixed the spacing issues so it should be ready for check-in.
Attachment #8380163 -
Attachment is obsolete: true
Attachment #8380237 -
Flags: checkin+
Comment 11•11 years ago
|
||
Comment on attachment 8380237 [details] [diff] [review] 974097 v2.patch We use checkin-needed for this. Thanks for the patch and welcome to Gecko hacking!
Attachment #8380237 -
Flags: checkin+
Updated•11 years ago
|
Keywords: checkin-needed
Updated•11 years ago
|
Attachment #8380237 -
Flags: review+
Comment 12•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/72591882de26
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/72591882de26
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in
before you can comment on or make changes to this bug.
Description
•