Closed
Bug 698263
Opened 13 years ago
Closed 13 years ago
Rename mozilla::imagelib namespaces to mozilla::image
Categories
(Core :: Graphics: ImageLib, defect)
Core
Graphics: ImageLib
Tracking
()
RESOLVED
FIXED
mozilla12
People
(Reporter: emorley, Assigned: atulagrwl, Mentored)
References
Details
(Whiteboard: [good first bug])
Attachments
(1 file, 3 obsolete files)
40.07 KB,
patch
|
Details | Diff | Splinter Review |
Bug 66984 moved ImageLib to image/ to get rid of the controversial name. As part of the discussion on tree location, it was suggested the namespace should match... Bobby Holley (:bholley), bug 66984 comment #26: > Just out of curiosity, why not imagelib? We've been namespacing new files as > mozilla::imagelib, so it might be nice if they matched. > > 'image' does have aesthetic appeal though, so I'm happy sticking with that > too. ;-) Joe Drew (:JOEDREW!), bug 66984 comment #27: > We should fix that by changing mozilla::imagelib to mozilla::image :) For anyone wanting to do this as a first bug: 1) Get mozilla-central: https://developer.mozilla.org/En/Developer_Guide/Source_Code/Mercurial 2) Rename the imagelib namespaces to image: http://mxr.mozilla.org/mozilla-central/search?string=imagelib&case=on 3) Check everything still builds ok: https://developer.mozilla.org/En/Simple_Firefox_build 4) Create a patch to attach here: https://developer.mozilla.org/En/Developer_Guide/How_to_Submit_a_Patch https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3f 5) Set the r? flag to request review from Joe Drew when you attach the patch (type in :JOEDREW and it will auto-complete). If you have any questions (or would like to take on this bug and don't have the needed bugzilla permissions to assign it to yourself), just ask! :-)
Flags: in-testsuite-
Updated•13 years ago
|
Assignee: nobody → abhishekkumarsingh.cse
Comment 1•13 years ago
|
||
Attachment #572187 -
Flags: review?(joe)
Updated•13 years ago
|
Attachment #572187 -
Flags: review?(joe) → review?(bmo)
Reporter | ||
Comment 2•13 years ago
|
||
Comment on attachment 572187 [details] [diff] [review] imagelib namespaces is changed to image Joe was the correct person to ask review from here, I'm not an imagelib peer. The review was requested over the weekend, so Joe will not have seen it yet. (The average time for reviews varies, but a few days-a week is probably the rough ballpark to expect).
Attachment #572187 -
Flags: review?(bmo) → review?(joe)
Comment 3•13 years ago
|
||
Comment on attachment 572187 [details] [diff] [review] imagelib namespaces is changed to image Review of attachment 572187 [details] [diff] [review]: ----------------------------------------------------------------- I've marked below (with "here") places you should *not* s/imagelib/image/; mostly these are in comments. I think it's helpful to be able to refer to "imagelib" itself, which is shorthand for "mozilla::image" from now on. ::: content/base/src/nsObjectLoadingContent.h @@ +362,5 @@ > UpdateFallbackState(nsIContent* aContent, AutoFallback& fallback, > const nsCString& aTypeHint); > > /** > + * The final listener to ship the data to (image, uriloader, etc) here ::: image/decoders/nsPNGDecoder.cpp @@ +602,5 @@ > png_read_update_info(png_ptr, info_ptr); > decoder->mChannels = channels = png_get_channels(png_ptr, info_ptr); > > /*---------------------------------------------------------------*/ > + /* copy PNG info into image structs (formerly png_set_dims()) */ here ::: image/public/ImageErrors.h @@ +39,5 @@ > > #include "nsError.h" > > /** > + * image specific nsresult success and error codes here ::: image/public/imgILoader.idl @@ +109,5 @@ > * @param aObserver the observer (may be null) > * @param cx some random data > * @param aListener [out] > * A listener that you must send the channel's notifications and data to. > + * Can be null, in which case image has found a cached image and is here ::: image/src/SVGDocumentWrapper.h @@ +184,5 @@ > bool mIgnoreInvalidation; > bool mRegisteredForXPCOMShutdown; > > // Lazily-initialized pointer to nsGkAtoms::svg, to make life easier in > + // non-libxul builds, which don't let us reference nsGkAtoms from image. here ::: image/src/imgLoader.cpp @@ +1782,5 @@ > > // If we're loading off the network, explicitly don't notify our proxy, > // because necko (or things called from necko, such as imgCacheValidator) > // are going to call our notifications asynchronously, and we can't make it > + // further asynchronous because observers might rely on image completing here @@ +1913,5 @@ > // Explicitly don't notify our proxy, because we're loading off the > // network, and necko (or things called from necko, such as > // imgCacheValidator) are going to call our notifications asynchronously, > // and we can't make it further asynchronous because observers might rely > + // on image completing its work between the channel's OnStartRequest and here ::: image/src/imgRequest.cpp @@ +1121,5 @@ > rv = nsMemory::HeapMinimize(true); > rv |= rasterImage->SetSourceSizeHint(sizeHint); > // If we've still failed at this point, things are going downhill > if (NS_FAILED(rv)) { > + NS_WARNING("About to hit OOM in image!"); here ::: layout/base/nsDocumentViewer.cpp @@ +1017,5 @@ > mLoaded = true; > > // Now, fire either an OnLoad or OnError event to the document... > bool restoring = false; > + // XXXbz image kills off the document load for a full-page image with here ::: layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp @@ +3338,5 @@ > if (isRTL) > twistyRect.x = rightEdge - twistyRect.width; > // yeah, I know it says we're drawing a background, but a twisty is really a fg > // object since it doesn't have anything that gecko would want to draw over it. Besides, > + // we have to prevent image from drawing it. here ::: netwerk/protocol/http/nsHttpChannel.cpp @@ +1215,5 @@ > > ClearBogusContentEncodingIfNeeded(); > > // this must be called before firing OnStartRequest, since http clients, > + // such as image, expect our cache entry to already have the correct here ::: xulrunner/config/mozconfig @@ +1,4 @@ > # This file specifies the build flags for XULRunner. You can use it by adding: > # . $topsrcdir/xulrunner/config/mozconfig > # to the top of your mozconfig file. > +ac_add_options --enable-debug and drop this.
Updated•13 years ago
|
Attachment #572187 -
Flags: review?(joe) → review+
Comment 4•13 years ago
|
||
Keyword:checkin-needed
Updated•13 years ago
|
Keywords: checkin-needed
Reporter | ||
Comment 5•13 years ago
|
||
Hi Abhishek :-) The changes mentioned in comment 3 need to be made before this can be checked in. It's common for reviewers to give r+ (ie grant review), but still list a few minor things that need to be changed first. These changes should still be made, the r+ just means they are happy for the updated patch to be attached (and then checkin-needed added), without another review. Whilst making those changes, please can you also see here for how to add author/commit message: http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed
Status: NEW → ASSIGNED
Keywords: checkin-needed
Reporter | ||
Comment 6•13 years ago
|
||
Abhishek if you need any help doing the things in comment 5 - just ask :-)
Assignee | ||
Comment 7•13 years ago
|
||
Done using two shell commands and finally manually verifying the changes grep -lr -r '::imagelib' *| xargs sed -i 's/::imagelib/::image/g' grep -lr -r 'namespace imagelib' *| xargs sed -i 's/namespace imagelib/namespace image/g' https://tbpl.mozilla.org/?tree=Try&rev=62632a303039
Assignee: abhishekkumarsingh.cse → atulagrwl
Attachment #572187 -
Attachment is obsolete: true
Attachment #575454 -
Flags: review?(joe)
Assignee | ||
Updated•13 years ago
|
Attachment #575454 -
Attachment is obsolete: true
Attachment #575454 -
Flags: review?(joe)
Assignee | ||
Comment 8•13 years ago
|
||
Abhishek, Sorry for taking the bug. I am assigning the bug back to you. Please make changes as suggested by reviewer and edmorley and let us know if you need any help.
Assignee | ||
Comment 9•13 years ago
|
||
Abhishek, Sorry for taking the bug. I am assigning the bug back to you. Please make changes as suggested by reviewer and edmorley and let us know if you need any help.
Assignee: atulagrwl → abhishekkumarsingh.cse
Assignee | ||
Updated•13 years ago
|
Attachment #572187 -
Attachment is obsolete: false
Reporter | ||
Comment 10•13 years ago
|
||
Atul, all yours. (I think enough time has passed now)
Assignee: abhishekkumarsingh.cse → atulagrwl
Assignee | ||
Comment 11•13 years ago
|
||
grep -lr -r '::imagelib' *| xargs sed -i 's/::imagelib/::image/g' grep -lr -r 'namespace imagelib' *| xargs sed -i 's/namespace imagelib/namespace image/g' https://tbpl.mozilla.org/?tree=Try&rev=38bd9c96e184
Attachment #572187 -
Attachment is obsolete: true
Attachment #583843 -
Flags: review?(joe)
Comment 12•13 years ago
|
||
Comment on attachment 583843 [details] [diff] [review] Patch v1.10 Lovely! I'll push this to try; once it's come back, we'll get someone to check it in.
Attachment #583843 -
Flags: review?(joe) → review+
Comment 13•13 years ago
|
||
Try run for 48e95de65ce6 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=48e95de65ce6 Results (out of 14 total builds): success: 14 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/jdrew@mozilla.com-48e95de65ce6
Updated•13 years ago
|
Keywords: checkin-needed
Comment 14•13 years ago
|
||
applying https://bug698263.bugzilla.mozilla.org/attachment.cgi?id=583843 patching file image/encoders/ico/nsICOEncoder.cpp Hunk #1 FAILED at 40 1 out of 1 hunks FAILED -- saving rejects to file image/encoders/ico/nsICOEncoder.cpp.rej patching file image/src/Decoder.cpp Hunk #1 FAILED at 36 1 out of 2 hunks FAILED -- saving rejects to file image/src/Decoder.cpp.rej patching file image/src/SVGDocumentWrapper.h Hunk #1 FAILED at 54 1 out of 2 hunks FAILED -- saving rejects to file image/src/SVGDocumentWrapper.h.rej patching file image/src/imgLoader.cpp Hunk #1 FAILED at 89 1 out of 1 hunks FAILED -- saving rejects to file image/src/imgLoader.cpp.rej patching file image/src/imgRequest.cpp Hunk #1 FAILED at 86 Hunk #2 FAILED at 132 2 out of 2 hunks FAILED -- saving rejects to file image/src/imgRequest.cpp.rej abort: patch failed to apply
Keywords: checkin-needed
Assignee | ||
Comment 15•13 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=b35b1d258237
Attachment #583843 -
Attachment is obsolete: true
Comment 16•13 years ago
|
||
Applying https://bug698263.bugzilla.mozilla.org/attachment.cgi?id=583843: patching file image/src/RasterImage.cpp Hunk #1 FAILED at 65 Hunk #2 succeeded at 169 (offset 2 lines). Hunk #3 succeeded at 2949 (offset 4 lines). 1 out of 3 hunks FAILED -- saving rejects to file image/src/RasterImage.cpp.rej Atul, please update and push to try. And be sure to mark this checkin-needed once the try results are in, so it doesn't get bit-rotted again! Thanks!
Comment 17•13 years ago
|
||
The patch applies fine with fuzz 6; probably no need for Atul upload a new patch. I've got the fuzz-applied patch locally, & assuming it builds OK locally, I'll push to try & then m-i today.
Comment 18•13 years ago
|
||
(FWIW, the chunk that doesn't apply without fuzz is just for some #includes changing in contextual code, a few lines up from a "using namespace mozilla::image[lib];" line.)
Comment 19•13 years ago
|
||
Re-pushed to Try for good measure: https://tbpl.mozilla.org/?tree=Try&rev=66512cdeb0e9 I also verified that the diffs between the r+'d patch & latest patch are all from bitrot in contextual code (good).
Comment 20•13 years ago
|
||
Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/9cf740464cd0 Thanks again, Atul & Abhishek!
Target Milestone: --- → mozilla12
Reporter | ||
Comment 21•13 years ago
|
||
Thanks for the patch! :-) https://hg.mozilla.org/mozilla-central/rev/9cf740464cd0
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Reporter | ||
Updated•10 years ago
|
Mentor: emorley
Whiteboard: [good first bug] [mentor=edmorley] → [good first bug]
You need to log in
before you can comment on or make changes to this bug.
Description
•