Open Bug 1805789 Opened 2 years ago Updated 2 years ago

Consider using PMSetDuplex/PMGetDuplex instead of using the NSDictionary key

Categories

(Core :: Printing: Setup, enhancement)

enhancement

Tracking

()

People

(Reporter: jwatt, Unassigned, NeedInfo)

Details

Attachments

(1 file)

In bug 1659856 part 3, jfkthame added duplex printing support to macOS.

Instead of using the "com_apple_print_PrintSettings_PMDuplexing" NSDictionary key in nsPrintSettingsX.mm we could use PMSetDuplex/PMGetDuplex.

Jonathan, do you recall if there was a reason to prefer the dictionary key? This came up while I was looking at bug 1805784 where I wondered if there might be some magic that goes on in PMSetDuplex/PMGetDuplex that we miss by setting the key directly (I don't have any evidence that that is the case).

Flags: needinfo?(jfkthame)

Hmm, curious. In the original version of bug 1659856 part 3, we did use PM{Set,Get}Duplex; see https://hg.mozilla.org/integration/autoland/rev/b61a789151df. But that was backed out, and when it re-landed as https://hg.mozilla.org/integration/autoland/rev/5d6896aa46ad, it had changed to use the dictionary key. I wonder why.....?

Ah... see https://phabricator.services.mozilla.com/D87960#2758440.

Flags: needinfo?(jfkthame)

Interesting, thanks for digging that up! I had no recollection of that conversation. I really should have requested a code comment explaining why we're not using the PM functions at the time I guess. :-)

I'm still not clear why we'd end up with settings being overwritten in that case. I guess we'd need to know which tests fail to figure that out.

It also makes me wonder whether the place where we call updateFromPMPrintSettings currently could potentially be the cause of some of our "setting X doesn't work with the native print dialog" bug reports.

Flags: needinfo?(jwatt)

For the record, it was Diff 4 in the link jfkthame gave that contained the PM function variant of the patch:

https://phabricator.services.mozilla.com/D87960?id=331108

And the definition of GetPMPrintSettings (since removed) can be found at:

https://searchfox.org/mozilla-central/rev/b81a2922acf198c2ef59fe292ee3bea234d1196f/widget/cocoa/nsPrintSettingsX.mm#202

You need to log in before you can comment on or make changes to this bug.