Closed Bug 92918 Opened 24 years ago Closed 24 years ago

Mozilla forces specific fonts for printer device

Categories

(Core :: Layout, defect)

Product:
Core
Component:
Layout
Platform:
All
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED WONTFIX
Milestone:
mozilla0.9.4

People

(Reporter: roland.mainz, Assigned: roland.mainz)

References

(
URL
)

Details

Attachments

(5 files)

Patch for 2001-07-27-08-trunk
1.55 KB, patch
Details | Diff | Splinter Review
New patch for 2001-07-27-08-trunk
3.24 KB, patch
Details | Diff | Splinter Review
New patch for 2001-07-27-08-trunk
3.29 KB, patch
Details | Diff | Splinter Review
New patch for 2001-07-31-08-trunk
5.50 KB, patch
Details | Diff | Splinter Review
New patch for 2001-07-31-08-trunk per dbaron's comments
4.61 KB, patch
Details | Diff | Splinter Review
Mozilla (layout!?) forces specific fonts for printer devices, even if those fonts do not match the properties for the selected printer (e.g. font does not match resolution and must be scaled up/down first). This is caused by the prefs "browser.display.use_document_fonts" - which instructs layout code to use _exactly_ those fonts requested by the document. Unfortunately this behaviour causes problems with a printer "device" like Xprint which offers both printer fonts and X11 bitmap fonts at the same time - layout prefers to choose the exact fonts (which are usually <=100DPI bitmap fonts; Xprint operates at >=300DPI) specified by the document - even if the font does not match. There is no workaround from within Xprint module code (mozilla/gfx/src/xprint/) - this needs to be fixed within layout code...
This is easy. Stealing bug... :-)
Assignee: karnaze → Roland.Mainz
Target Milestone: --- → mozilla0.9.4
Status: NEW → ASSIGNED
Keywords: patch, review
Filed patch. Tested with PostScript and Xprint modules on Linux+Solaris. Works perfectly. The patch simply replaces the pref "browser.display.use_document_fonts" with "printer.use_document_fonts" for printer device contexts - which defaults to PR_FALSE. sujay/karnaze/dcone, wanna r= this patch, please ?
I think patching requires you to patch agains most recent version (cvs)
Francisco León: That's not neccesary (AFAIK). The touched source does not change that frequently; per CVS log (http://bonsai.mozilla.org/cvslog.cgi?file=mozilla/layout/base/src/nsPresContext.cpp) the last change was from 24.07.2001 ...
1) Why is this an int pref? Would a bool pref not make more sense? For browser.display.use_document_fonts, we have: // 0 = never, 1 = quick, 2 = always Do we ever draw a distinction between "quick" and "always" for either of these two prefs? 2) If you add a new pref name, please add a default value for the pref to all.js
[1]= no clue. I did not write that code. I just fixed a issue here... :-) Unfortunately this cannot be changed in a simple way, tons of code use that prefs, even register pref callbacks for it. Touching all that code is far out of scope of this bug... [2]= good idea, new patch follows...
well... Here's the thing. browser.display.use_document_fonts is an int pref. Is there a reason (other than consistency) to make printer.use_document_fonts an int pref? After all, you're treating it as simply a bool pref. I'm not suggesting changing the browser.display.use_document_fonts, which _is_ outside the scope of this bug. If you feel that consistency between the two font prefs is important, that's fine. Just wanted to make sure you'd considered this. :)
Filed new patch per timeless commets (Smarter code :) The new patch now adds the new prefs to all.js (+an older one which wasn't included in all.js yet) ... OK... again... requesting r/sr/a=/etc.= ...
+ /* that should be mDeviceContext->IsPrinterDevice() in stead of mDsome day... */ what is mDsome? :) I would put the comment describing pref("print.print_method", 0); in /* */ and the actual pref _after_ the comment preceded by // that makes it easier to uncomment the pref.... is there a reason that's commented at the moment anyway?
> + /* that should be mDeviceContext->IsPrinterDevice() in stead of mDsome > day... */ > what is mDsome? :) I file a new patch... > I would put the comment describing pref("print.print_method", 0); in /* */ and > the actual pref _after_ the comment preceded by // Ok... > that makes it easier to uncomment the pref.... is there a reason that's > commented at the moment anyway? Erm... READ the comment. Setting that prefs would kill autoselection of print system. The autodetection works only _only_ if GetIntPrefs() fails ...
OK... new patch, requesting r=/sr= and so on...
r=timeless, probably drop () from (!snw)
Keywords: review → approval
Thanks. I don't think it's worth to file a new patch for that to-much-safety-bracketsin-code issue. I'll file a new patch for checkin or if the superreviewer finds more issues to fix... :-) Requesting sr= ...
Blocks: 88554
I want reviews from dcone and dbaron before I will super-review this. Is this the right way to do this? Also, you need module owner review, too. In this case it's one of the layout team.
Doesn't this just turn off any document specification of the fonts for printing? That seems like the wrong thing (TM). (Well, I think it would if that pref worked -- I'm not sure whether it does.) Also, if you want to override something just for printing, can't you do it in nsPrintContext (which inherits from nsPresContext, as does nsGalleyContext)? Finally, if you end up checking something like this in, could you replace that horrible mUseDocumentFonts = prefInt == 0 ? PR_FALSE : PR_TRUE; with something nicer such as mUseDocumentFonts = (prefInt != 0);
> Doesn't this just turn off any document specification of the fonts for > printing? > That seems like the wrong thing (TM). (Well, I think it would if that pref > worked -- I'm not sure whether it does.) No... it isn't really wrong - PostScript module usually does not have any fonts specified by the documents - in this case layout searches for fonts with similar attributes. AFAIK PostScript module won't be affected by this patch. Xprint is another world. It offers far more fonts - including those X11 scaleable and _bitmap_ fonts (those are provided as "last" option... fallback if everything else goes wrong (e.g. no matching buildin printer font and no postscript font which could be downloaded to the printer)). Unfortunately... layout somehow "sees" those ~90DPI bitmap fonts in the font list of Xprt and _demands_ to get _exactly_ those fonts - regardless of the fact that they have to be scaled before usage... > Also, if you want to override something just for printing, can't you do it in > nsPrintContext (which inherits from nsPresContext, as does nsGalleyContext)? Would be one theoretiocal option. But that involves far more changes... adds more complexity... The current patch keeps things together in a simple way. Is it really mandatory to override nsPresContext::GetUserPreferences() in nsPrintContext just for that single functionality ? Sounds like overkill for me... Finally... I'd like to ship this fix in 0.9.4 . and I am not sure whether it is possible to catch that milestone, r=/sr= for crossplatform bugs is more than slow (my experience: two months and more starting with the date of the 1st patch). > Finally, if you end up checking something like this in, could you replace that > horrible > mUseDocumentFonts = prefInt == 0 ? PR_FALSE : PR_TRUE; > with something nicer such as > mUseDocumentFonts = (prefInt != 0); Why is that "horrible" ? Anyway... this is easy to fix. More problems to fix for r= ?
That's ridiculous. On Windows, all system fonts are available when printing, as well as the printer's fonts (hopefully). I presume Mac is at least somewhat similar. With Postscript, things are more limited, but really they don't have to be -- things *ought* to be able to work when the system has adequate scalable fonts. If we're not able to print fonts properly when they are available, then that's a bug that should be filed separately. And for that matter, if there are problems, web pages can specify print media stylesheets, and we shouldn't break those stylesheets. I think the GTK font code has some code involving tolerance factors for scaling bitmap fonts. You should probably emulate that in the xprint code, so that you choose a font that the printer has rather than scale a bitmap if the scaling were out of a certain range (you could even have a tolerance pref defaulting to 0 if you want). I think a fix based on scaling-tolerance is the right way to fix this bug.
Xprint code uses Xlib-port code - which is a clone from GTK+-toolikit code. Unfortunately, it looks that layout is the only place to fix this. Those bitmap fonts are queried via name - which makes it impossible to catch that and redirect it to another font. Disabling bitmap fonts is no option, too - this would break i18n font support (e.g. the described fallback option which is used when no other option is left - this even allows Xprint to print fonts which are not installed on the printer). Ok... does anyone see an option here - except that solution in my patch ?
hard code a check for unix. afaik the current complaints are that you're mucking w/ the behavior on non unix printing platforms. and clarify your check for xprint, you said you were checking for xprint, but you aren't really...
timeless: Oh... yes... I was blind. Simple solution... :-) Thanks ! I'll file a new patch in a few hours... need some sleep first...
I still don't think that's the right solution. I think you should have a scaling threshhold. Then, when scalable fonts are available, you could actually *support* the document's fonts when using xprint (which would be an advantage over Postscript).
And I don't think a scaling threshhold would break i18n. You should just only reject a font based on the scaling threshhold if you know you have a scalable font that will render the same characters.
> Is it really mandatory to override nsPresContext::GetUserPreferences() in > nsPrintContext just for that single functionality ? Well... that would preclude the need for your hack detection of whether it's a print context, would it not? It should be fairly simple. Have nsPrintContext::GetUserPreferences() call nsPresContext::GetUserPreferences() and then just override mUseDocumentFonts with your printer.use_document_fonts value. That said, I think dbaron's suggestion of limiting scaling is a better approach.
dbaron's idea of "reject" some fonts via "scaling threshhold" does not work because the font is requested per _name_. I tested this and tried various hacks. No way... ;-( It has to request another font name - that's the only working solution. And that's what my patch does (via |mUseDocumentFonts = PR_FALSE|).
Do I need to explain it a third time? [first on IRC, second here] > You should just only > reject a font based on the scaling threshhold if you know you have a scalable > font that will render the same characters.
I tested that. It does not work properly.
How do you know that wasn't due to a mistake in your implementation?
dbaron: I hacked a small test application around gfx/src/xlib stuff and tested it manually. Then I tested it with Mozilla - which still continued to demand on scaled bitmap fonts even if I offer "better fonts". Then I tried to find a good match algorithm and "rejected" font requests for bitmap fonts. That hack nuked most text at http://www.yahoo.cp.jp/ Finally I added a hack (see http://lxr.mozilla.org/seamonkey/source/gfx/src/xlib/nsFontMetricsXlib.cpp#2607) to make this situation a little bit better... but http://www.cnn.com/ still renders using scaled bitmap fonts.
Sorry... was http://www.yahoo.co.jp/ ...
URL: http://www.cnn.com/
Note that this issue is not Xprint-specific. It will happen on any X11 server which runns with a high DPI value (e.g. >=300DPI)...
Filed new patch. Changes: - The new functionality only affects Unix platforms (|#ifdef XP_UNIX| ...). No thread for Win*/Mac* anymore - |mUseDocumentFonts| is now set in PrintContext::GetUserPreferences() instead of nsPresContext::GetUserPreferences() - additionally I nuked the misuse of |nsIDeviceContext->SupportsNativeWidgets()| in nsCSSFrameConstructor.cpp and replaced it with |IsPaginated()|. Any comments ?
Despite that I think the entire patch is wrong, two nits: * moving the declaration of |nsresult rv| is silly - C++ style is not to declare variables until you need them. * Don't put unix-specific comments/prefs in all.js. Put them in unix.js.
Keywords: approval → review
dbaron: > Despite that I think the entire patch is wrong, Uhm... even that replacement of |SupportsNativeWidgets()| with |IsPaginated()| ? :-) ... ... but I agree with you, the patch is only a silly workaround. But I don't have a "gfx/src/xlib"-only-solution which doesn't break something else... there should be a general fix for this issue as it may affect all Xservers with high resolutions... Xprint uses Xlib-port code which is a clone from GTK+ port... I assume that Mozilla suffers in general from this issue... there should be a new bug to track this... > two nits: > * moving the declaration of |nsresult rv| is silly - C++ style is not to > declare variables until you need them. Oups... artifact from old patch... sorry... Will be fixed... > * Don't put unix-specific comments/prefs in all.js. Put them in unix.js. Will be fixed...
OK... is there any problem with this... uhm... workaround ? Or is it "ripe" to get it's r= ... ?
OK... requesting r= for the new patch, please...
CC:'ing more layout people for comments/help/review...
<gisburn_> do you want to r= a layout bug ? <waterson> what bug? <gisburn_> bug 92918 <waterson> don't use /* */ comments in this file. nobody else does. <waterson> explain to me what this patch does please. <gisburn_> OK... problem: It seems that layout chooses low-resolution bitmap fonts (for example... 75DPI font) on a 300DPI/600DPI X11 server - which results in bad font quality. <gisburn_> the patch works around that issue by turning use_document_font off ... <gisburn_> ... for unix print modules ... <gisburn_> ... which affects PostScript module and Xprint module... <waterson> why does dbaron think that this patch is completely wrong? <waterson> (it sure looks like a hack to me.) <gisburn_> Erm... a "better" way would be to catch the real problem: Find out the real reason why layout demands on bitmap fonts. <gisburn_> The nproblem is that the fonts are requested by name... e.g. I cannot catch this in gfx/src/xlib code <gisburn_> therefore I have to find a workaround... <gisburn_> ... that#s that patch... <waterson> because you can't figure out why layout demands a bitmapped font? <gisburn_> yup... that's my problem. <waterson> well, i'm not going to r= this patch because you can't figure out why layout wants bitmapped fonts. <gisburn_> broken layout ?
Waterson: Thanks! ;-( Per IRC comments: Marking bug as WONTFIX, adding comment about broken font support to docs.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → WONTFIX
Ahem. Gisburn, how about opening a bug on layout wanting bitmapped fonts when it ought not? /be
No longer blocks: 88554
Filed bug 93771 to get a solution for this someday...
BTW: I filed bug 93830 ("Remove IsItAPrinter-hack from nsCSSFrameConstructor.cpp") to get rid of the "SupportsnativeWidgets()==true to test if device is a printer"-hack in nsCSSFrameConstructor.cpp...
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: