GetPath() impls in nsDeviceContextSpec* are unused/useless & should be removed

RESOLVED FIXED in mozilla38



4 years ago
4 years ago


(Reporter: dholbert, Assigned: dholbert)


Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)



(1 attachment)



4 years ago
Several nsDeviceContextSpec[PLATFORM] implementations have this method...
 NS_IMETHOD GetPath (const char** aPath);
...which looks like it's implementing an interface method, but is not.  (and hence is unused externally)

MXR search showing this code:

These methods can all go away.

HISTORICAL NOTE (feel free to ignore):
It looks like this dates back to this change in nsDeviceContextSpecG...
...which was just adding a bunch of accessors (in a needlessly-XPCOM-flavored way), AFAICT. And then nsDeviceContextSpecAndroid and nsDeviceContextSpecQt cribbed from that.

Comment 1

4 years ago
Created attachment 8551942 [details] [diff] [review]
fix v1
Attachment #8551942 - Flags: review?(roc)

Comment 2

4 years ago
(nsDeviceContextSpecGTK::GetPath() has one caller -- GetSurfaceForPrinter -- but that's another nsDeviceContextSpecGTK method, which can just use 'mPath' instead of calling GetPath, as shown in the patch.)

Comment 3

4 years ago
(FWIW, I found this bug while annotating all of the "NS_IMETHOD" methods in nsDeviceContextSpecG.h as MOZ_OVERRIDE. That triggered a compile error for GetPath(), because it doesn't actually override anything.)


4 years ago
Blocks: 1123844


4 years ago
No longer blocks: 1123844

Comment 5

4 years ago
Comment on attachment 8551942 [details] [diff] [review]
fix v1

karl, would you be up for reviewing this? I just noticed he's got quite a review queue, so it'd be better if I could farm this out onto someone else. (I originally tagged him since this is nominally a cross-/widget change, but really Widget|GTK is the only place this method is used, and the other impls seem to just be cribbed from that & unused.)
Attachment #8551942 - Flags: review?(roc) → review?(karlt)

Comment 6

4 years ago
s/noticed he's/noticed roc's/
The checkin comment, with no bug, was
"Added UNIX support for postscript printing, fixed many bugs.
 Added nsIDeviceContextSpecPS interface so we can do PS on any platoform, get
 information from the print dialog."

I think these missed being included in --disable-postscript in bug 80625 and
so didn't get removed at the same time as removal of nsDeviceContextSpecPS in
these changesets
Depends on: 383889
Attachment #8551942 - Flags: review?(karlt) → review+

Comment 8

4 years ago
Ah, good observation! That second cset (693f69afaff5) did remove an invocation of GetPath() on a nsIDeviceContextSpecPS* pointer. That must be what this API was for.

Anyway -- thanks for the review & archeology! I'll land this when the tree reopens.
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.