Closed Bug 1185236 Opened 9 years ago Closed 7 years ago

Firefox unable to print page with long title using CUPS

Categories

(Core :: Printing: Setup, defect)

39 Branch
x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: tataranovich, Assigned: mantaroh)

References

Details

Attachments

(5 files)

User Agent: Mozilla/5.0 (X11; Linux i686; rv:39.0) Gecko/20100101 Firefox/39.0 Iceweasel/39.0
Build ID: 20150703101329

Steps to reproduce:

1) Open a page with long title, example: http://детский-мир.net/%D0%A0%D0%B0%D1%81%D0%BA%D1%80%D0%B0%D1%81%D0%BA%D0%B8/7932.htm 
2) Try to print it using CUPS printer



Actual results:

Firefox gives no error, nut no page printed. CUPS log (/var/log/cups/error_log) contain following error:

E [18/Jul/2015:16:20:40 +0300] [Client 17] Returning IPP client-error-attributes-or-values-not-supported for Print-Job (ipp://localhost:631/printers/HP_LaserJet_1018) from localhost


Expected results:

Page should be printed.
I find similar bug in Thunderbird (https://bugzilla.mozilla.org/show_bug.cgi?id=917340) - both bugs related to a long print jobname which is constructed from page title. If I reduce title length using developers tools - firefox will successfully print this page.
The same problem when the title-tag is empty and Firefox automatically uses the URL (in my case the length was 309).
By using firebug (or developer tools) and adding a short title inside the header-section, the Problem was solved.
User-Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:41.0) Gecko/20100101 Firefox/41.0
Build ID: 20151015172900
CUPS log: [Client 18] Returning IPP client-error-attributes-or-values-not-supported for Print-Job (ipp://localhost:631/printers/ML-1640-Series) from localhost
Status: UNCONFIRMED → NEW
Component: Untriaged → Printing: Setup
Ever confirmed: true
Product: Firefox → Core
https://tools.ietf.org/html/rfc2911
4.1.2 'name'

   This syntax type is used for user-friendly strings, such as a Printer
   name, that, for humans, are more meaningful than identifiers. Names
   are never translated from one natural language to another. The
   'name' attribute syntax is essentially the same as 'text', including
   the REQUIRED support of UTF-8 except that the sequence of characters
   is limited so that its encoded form MUST NOT exceed 255 (MAX) octets.

   Also like 'text', 'name' is really an abbreviated notation for either
   'nameWithoutLanguage' or 'nameWithLanguage'. That is, all IPP
   objects and clients MUST support both the 'nameWithoutLanguage' and
   'nameWithLanguage' attribute syntaxes. However, in actual usage and
html file for reproducing problem
I´m not sure if this belongs here but the same problem exist also in Thunderbird see here:

https://bugzilla.mozilla.org/show_bug.cgi?id=917340
This bug still exists in FF47, I'm currently updating the system and will confirm it for FF49.

The solution seems to be according to the RFC above to truncate the name field to 255 characters.
Not only is Unix, but macOS also has the same problem.
The IPP disallow over 256 characters for job-title[1], but Gecko passes the long title.
Blink and Webkit cuts this title length[2][3][4].

Note:
1) Gecko cuts 255 lengths on windows[5].
2) GNOME Evince has the same problem(i.e. we can't print long title via IPP). I think that Evince should cut title as well.

[1] https://tools.ietf.org/html/rfc2911#section-3.2.1.1
[2] https://bugs.chromium.org/p/chromium/issues/detail?id=65469
[3] https://chromium.googlesource.com/experimental/chromium/src/+/refs/wip/bajones/webvr_1/printing/printing_utils.cc#21
[4] https://github.com/WebKit/webkit/blob/c637754ee7dfca6142e441d2040026903fee0c1f/Source/WebKit/WebProcess/WebPage/gtk/WebPrintOperationGtk.cpp#L83
[5] http://searchfox.org/mozilla-central/rev/51eae084534f522a502e4b808484cde8591cb502/widget/windows/nsDeviceContextSpecWin.cpp#468
OS: Unspecified → Mac OS X
Hardware: Unspecified → x86
(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #9)
> 1) Gecko cuts 255 lengths on windows[5].
....
> [5]
> http://searchfox.org/mozilla-central/rev/
> 51eae084534f522a502e4b808484cde8591cb502/widget/windows/
> nsDeviceContextSpecWin.cpp#468

Oh, this is skia version.

Windows is here(Cairo):
https://dxr.mozilla.org/mozilla-central/rev/f9a5e9ed62103c84e4cde915f4d08f1ce71be83e/gfx/thebes/PrintTargetWindows.cpp#66-70

In any way, windows already shorten the title.
Thank you for the links.

There are two bugs reported here, on Linux:

1) The failure to print.

2) The lack of any error message.

The first bug is a problem with the GTK cups print backend, where is generates
the IPP name.  (Gecko uses gtk_print_job_new(), for which the title parameter
is not necessarily associated with IPP and has no length restrictions in fixed
versions.)
The initial report https://bugzilla.gnome.org/show_bug.cgi?id=755988
was fixed in
https://git.gnome.org/browse/gtk+/log/gtk/gtkprintoperation.c?h=3.18.2
The fixed was moved to the cups implementation for
https://bugzilla.gnome.org/show_bug.cgi?id=774097
(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #9)
> Not only is Unix, but macOS also has the same problem.

Have you reproduced on MacOS?
Is there an error message?

https://developer.apple.com/documentation/applicationservices/1460149-pmprintsettingssetjobname?language=objc
doesn't say anything about IPP.

> Note:
> 1) Gecko cuts 255 lengths on windows[5].

https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx
indicates that Gecko is cutting to 259 octets on WINNT.

> 2) GNOME Evince has the same problem(i.e. we can't print long title via
> IPP). I think that Evince should cut title as well.

That's the GTK cups print backend bug.
Comment on attachment 8906928 [details]
Bug 1185236 - Shorten print job name when GTK version is older than 3.18.2.

https://reviewboard.mozilla.org/r/178650/#review184350

I'm happy to take a workaround for older GTK versions, but please restrict the
workaround to those versions, and please don't split UTF-8 multi-byte
characters as this has the potential to confuse other consumers.
GLib provides g_utf8_find_prev_char(), which could be useful.
I don't know whether or not Gecko has a similar utility.

We should get someone else to review a Cocoa change, so please do that in a
separate patch.
Attachment #8906928 - Flags: review?(karlt)
(In reply to Karl Tomlinson (:karlt) from comment #14)
> (In reply to Mantaroh Yoshinaga[:mantaroh] from comment #9)
> > 1) Gecko cuts 255 lengths on windows[5].
> 
> https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx
> indicates that Gecko is cutting to

259 UTF-16 codes, actually.
(In reply to Karl Tomlinson (:karlt) from comment #14)
> (In reply to Mantaroh Yoshinaga[:mantaroh] from comment #9)
> > Not only is Unix, but macOS also has the same problem.
> 
> Have you reproduced on MacOS?
> Is there an error message?
> 
> https://developer.apple.com/documentation/applicationservices/1460149-
> pmprintsettingssetjobname?language=objc
> doesn't say anything about IPP.
> 

Yes, I got the following messages in cups error message with CUPS debug mode:

D [14/Sep/2017:10:57:42 +0900] Create-Job ipp://localhost:631/printers/Canon_MF510_Series
D [14/Sep/2017:10:57:42 +0900] Create-Job client-error-attributes-or-values-not-supported: Bad job-name value: “job-name”: 名前値“Title with more than 255 characters (256) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++”が正しくありません - 長さ256が正しくありません(RFC 2911 セクション4.1.2)。
E [14/Sep/2017:10:57:42 +0900] [Client 83] Returning IPP client-error-attributes-or-values-not-supported for Create-Job (ipp://localhost:631/printers/Canon_MF510_Series) from localhost

As far as I can see in the open radar, there are not same bugs.
Comment on attachment 8906928 [details]
Bug 1185236 - Shorten print job name when GTK version is older than 3.18.2.

https://reviewboard.mozilla.org/r/178650/#review184350

Thank you so much!

OK. I'll do that.
(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #17)
> 
> As far as I can see in the open radar, there are not same bugs.

Filed, Radar: https://openradar.appspot.com/34428043
(In reply to Karl Tomlinson (:karlt) from comment #16)
> (In reply to Karl Tomlinson (:karlt) from comment #14)
> > (In reply to Mantaroh Yoshinaga[:mantaroh] from comment #9)
> > > 1) Gecko cuts 255 lengths on windows[5].
> > 
> > https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx
> > indicates that Gecko is cutting to
> 
> 259 UTF-16 codes, actually.

Oh, Windows has the same problem as well. I can reproduce via ipp print.
I think that we should cut job-name length on windows. I confirmed prevent this phenomenon if we will cut job-name length.
Comment on attachment 8913052 [details]
Bug 1185236 - Shorten print job name on Cocoa.

https://reviewboard.mozilla.org/r/180134/#review190074

::: widget/cocoa/nsDeviceContextSpecX.mm:140
(Diff revision 1)
> -        ::CFStringCreateWithCharacters(NULL, reinterpret_cast<const UniChar*>(aTitle.BeginReading()),
> +      ::CFStringCreateWithCharacters(NULL, reinterpret_cast<const UniChar*>(aTitle.BeginReading()),
> -                                             aTitle.Length());
> +                                           title.Length());

Using Length of the C string with the UTF16 string doesn't look right.
Comment on attachment 8913051 [details]
Bug 1185236 - Add adjustPrintJobName in order to avoid IPP job name limit.

https://reviewboard.mozilla.org/r/180132/#review190032

::: gfx/thebes/PrintTarget.h:19
(Diff revision 1)
>  namespace mozilla {
>  namespace gfx {
>  
> +// IPP spec disallow the job-name which is over 256 characters.
> +// RFC: https://tools.ietf.org/html/rfc2911#section-4.1.2
> +#define MAX_JOB_NAME_LENGTH 255

MAX_IPP_JOB_NAME_LENGTH would be a little clearer.
If this won't be needed outside the .cpp file, then please move the define to the .cpp file.

::: gfx/thebes/PrintTarget.h:143
(Diff revision 1)
>     * returned DrawTarget it is still valid to use after EndPage() has been
>     * called.
>     */
>    virtual already_AddRefed<DrawTarget> GetReferenceDrawTarget(DrawEventRecorder* aRecorder);
>  
> +  static void AdjustPrintJobName(const nsAString& aJobName,

Please name this AdjustPrintJobNameForIPP or AdjustJobNameForIPP.

::: gfx/thebes/PrintTarget.cpp:168
(Diff revision 1)
> +/* static */
> +void
> +PrintTarget::AdjustPrintJobName(const nsAString& aJobName,
> +                                nsCString& aAdjustedJobName)
> +{
> +  nsCString title = NS_ConvertUTF16toUTF8(aJobName);

NS_ConvertUTF16toUTF8 is a class, and so this would be

NS_ConvertUTF16toUTF8 title(aJobName);

That looks odd to me though, and so I suggest

nsCString title;
AppendUTF16toUTF8(aJobName, title);

Another option is:

CopyUTF16toUTF8(aJobName, aAdjustedJobName);

::: gfx/thebes/PrintTarget.cpp:172
(Diff revision 1)
> +    do {
> +      title.SetLength(length--);
> +    } while (!IsUTF8(title) && length > 0);

RewindToPriorUTF8Codepoint() should be a bit more efficient here, and may even be a little simpler.

::: gfx/thebes/PrintTarget.cpp:175
(Diff revision 1)
> +    if (!IsUTF8(title)) {
> +      title = NS_LITERAL_CSTRING("Mozilla Document");

As title is from a conversion to UTF8, it should be safe to assume that it is valid UTF8, and so this path is not necessary.
Attachment #8913051 - Flags: review?(karlt)
Comment on attachment 8906928 [details]
Bug 1185236 - Shorten print job name when GTK version is older than 3.18.2.

https://reviewboard.mozilla.org/r/178650/#review190026

::: commit-message-bda52:3
(Diff revision 2)
> +Bug 1185236 - Shorten print job name when GTK version is older than 3.18.2. r?karlt
> +
> +Until GTK 3.18.2, GTK allow setting job name with exceeding 255 characters.

*Since* GTK 3.18.2, GTK *allows* setting job name with *more than* 255 *bytes*.

::: widget/gtk/nsDeviceContextSpecG.cpp:300
(Diff revision 2)
>                                        const nsAString& aPrintToFileName,
>                                        int32_t aStartPage, int32_t aEndPage)
>  {
>    mTitle.Truncate();
> -  AppendUTF16toUTF8(aTitle, mTitle);
> +
> +  // GTK sent print job with names exceeding 255 if GTK is older than 3.18.2.

Either "Print job names exceeding 255 bytes are safe with GTK version 3.18.2 or newer."
or "GTK versions older than 3.18.2 may error on print job names exceeding 255 bytes."

::: widget/gtk/nsDeviceContextSpecG.cpp:301
(Diff revision 2)
>                                        int32_t aStartPage, int32_t aEndPage)
>  {
>    mTitle.Truncate();
> -  AppendUTF16toUTF8(aTitle, mTitle);
> +
> +  // GTK sent print job with names exceeding 255 if GTK is older than 3.18.2.
> +  // This is workaround for old GTK.

"is *a* workaround"

::: widget/gtk/nsDeviceContextSpecG.cpp:302
(Diff revision 2)
>  {
>    mTitle.Truncate();
> -  AppendUTF16toUTF8(aTitle, mTitle);
> +
> +  // GTK sent print job with names exceeding 255 if GTK is older than 3.18.2.
> +  // This is workaround for old GTK.
> +  nsCString title;

Pass mTitle directly, so that |title| is not required.

::: widget/gtk/nsDeviceContextSpecG.cpp:306
(Diff revision 2)
> +  // This is workaround for old GTK.
> +  nsCString title;
> +  if (gtk_check_version(3,18,2) != nullptr) {
> +    PrintTarget::AdjustPrintJobName(aTitle, title);
> +  } else {
> +    title = NS_ConvertUTF16toUTF8(aTitle);

CopyUTF16toUTF8(aTitle, mTitle)

And the Truncate() above can be removed.
Attachment #8906928 - Flags: review?(karlt)
Comment on attachment 8913051 [details]
Bug 1185236 - Add adjustPrintJobName in order to avoid IPP job name limit.

https://reviewboard.mozilla.org/r/180132/#review190032

Thank you for review this, and sorry for late reply.
I've fixed this. Could you please review this again?

> MAX_IPP_JOB_NAME_LENGTH would be a little clearer.
> If this won't be needed outside the .cpp file, then please move the define to the .cpp file.

OK, I've moved this definition to the cpp file.

> RewindToPriorUTF8Codepoint() should be a bit more efficient here, and may even be a little simpler.

I just got to know now! thanks.
Comment on attachment 8913052 [details]
Bug 1185236 - Shorten print job name on Cocoa.

https://reviewboard.mozilla.org/r/180134/#review190074

> Using Length of the C string with the UTF16 string doesn't look right.

Oh, sorry.
This is totally my mistake. I'd like to use title here. I've fixed this.
Comment on attachment 8913052 [details]
Bug 1185236 - Shorten print job name on Cocoa.

https://reviewboard.mozilla.org/r/180134/#review191994

::: widget/cocoa/nsDeviceContextSpecX.mm:140
(Diff revision 2)
> -        ::CFStringCreateWithCharacters(NULL, reinterpret_cast<const UniChar*>(aTitle.BeginReading()),
> -                                             aTitle.Length());
> +      ::CFStringCreateWithCharacters(NULL,
> +                                     reinterpret_cast<const UniChar*>(title.get()),

|aTitle| was a UTF16 string, and so I assume UniChar is similar.  |title| is a C string, i.e. bytes.
It needs to be converted back to UTF16 before casting its data to UniChar.

I assume the WINNT port would benefit from something similar, and so I'd suggest two static methods on PrintTarget():

AdjustPrintJobNameForIPP(nsCString& aJobName);
AdjustPrintJobNameForIPP(nsString& aJobName);

The latter would use the former.
Comment on attachment 8913051 [details]
Bug 1185236 - Add adjustPrintJobName in order to avoid IPP job name limit.

https://reviewboard.mozilla.org/r/180132/#review191998

::: gfx/thebes/PrintTarget.cpp:20
(Diff revision 2)
>  #include "mozilla/gfx/2D.h"
>  #include "mozilla/gfx/HelpersCairo.h"
>  #include "mozilla/gfx/Logging.h"
> +#include "nsUTF8Utils.h"
> +
> +// IPP spec disallow the job-name which is over 256 characters.

The limit is 255 octets (or bytes).

::: gfx/thebes/PrintTarget.cpp:178
(Diff revision 2)
> +  CopyUTF16toUTF8(aJobName, aAdjustedJobName);
> +
> +  if (aAdjustedJobName.Length() > IPP_JOB_NAME_LIMIT_LENGTH) {
> +    uint32_t length =
> +      RewindToPriorUTF8Codepoint(aAdjustedJobName.get(),
> +                                 (uint32_t)(IPP_JOB_NAME_LIMIT_LENGTH - 3));

Please remove the (uint32_t) reinterpret_cast as an explicit cast is not necessary.
Attachment #8913051 - Flags: review?(karlt) → review+
Comment on attachment 8906928 [details]
Bug 1185236 - Shorten print job name when GTK version is older than 3.18.2.

https://reviewboard.mozilla.org/r/178650/#review192006

Thanks!

::: widget/gtk/nsDeviceContextSpecG.cpp:300
(Diff revision 3)
>                                        const nsAString& aPrintToFileName,
>                                        int32_t aStartPage, int32_t aEndPage)
>  {
> -  mTitle.Truncate();
> -  AppendUTF16toUTF8(aTitle, mTitle);
> +  // Print job names exceeding 255 bytes are safe with GTK version 3.18.2 or
> +  // newer. This is a workaround for old GTK.
> +  nsCString title;

Remove |title| as it is no longer used.
Attachment #8906928 - Flags: review?(karlt) → review+
Comment on attachment 8913051 [details]
Bug 1185236 - Add adjustPrintJobName in order to avoid IPP job name limit.

https://reviewboard.mozilla.org/r/180132/#review191998

Thanks!

> Please remove the (uint32_t) reinterpret_cast as an explicit cast is not necessary.

A second parameter of RewindToPriorUTF8Codepoint() defined as UnsignedT, So I think we need this cast.
Comment on attachment 8913052 [details]
Bug 1185236 - Shorten print job name on Cocoa.

https://reviewboard.mozilla.org/r/180134/#review191994

> |aTitle| was a UTF16 string, and so I assume UniChar is similar.  |title| is a C string, i.e. bytes.
> It needs to be converted back to UTF16 before casting its data to UniChar.
> 
> I assume the WINNT port would benefit from something similar, and so I'd suggest two static methods on PrintTarget():
> 
> AdjustPrintJobNameForIPP(nsCString& aJobName);
> AdjustPrintJobNameForIPP(nsString& aJobName);
> 
> The latter would use the former.

Thanks.
OK, I add AdjustPrintJobNameForIPP of wide version.
Comment on attachment 8916862 [details]
Bug 1185236 - Add AdjustPrintJobNameForIPP() wide version.

https://reviewboard.mozilla.org/r/187922/#review192986
Attachment #8916862 - Flags: review?(karlt) → review+
Comment on attachment 8913051 [details]
Bug 1185236 - Add adjustPrintJobName in order to avoid IPP job name limit.

https://reviewboard.mozilla.org/r/180132/#review191998

> A second parameter of RewindToPriorUTF8Codepoint() defined as UnsignedT, So I think we need this cast.

Ah, thanks.  How about replacing "3" with "3U", so that the result of the expression is unsigned even without the cast?
Comment on attachment 8913052 [details]
Bug 1185236 - Shorten print job name on Cocoa.

https://reviewboard.mozilla.org/r/180134/#review194214
Attachment #8913052 - Flags: review?(mstange) → review+
Comment on attachment 8913051 [details]
Bug 1185236 - Add adjustPrintJobName in order to avoid IPP job name limit.

https://reviewboard.mozilla.org/r/180132/#review191998

> Ah, thanks.  How about replacing "3" with "3U", so that the result of the expression is unsigned even without the cast?

Thanks!
I replaced it using by "3U".
(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #50)
> Comment on attachment 8906928 [details]
> Bug 1185236 - Shorten print job name when GTK version is older than 3.18.2.
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/178650/diff/5-6/

Oops, sorry. I forgot to add 'ForIPP' suffix on GTK code.
Pushed by mantaroh@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/445f1e6fc862
Add adjustPrintJobName in order to avoid IPP job name limit. r=karlt
https://hg.mozilla.org/integration/autoland/rev/0466471b9f67
Shorten print job name when GTK version is older than 3.18.2. r=karlt
https://hg.mozilla.org/integration/autoland/rev/2cbe403d5c16
Add AdjustPrintJobNameForIPP() wide version. r=karlt
https://hg.mozilla.org/integration/autoland/rev/eda76603d637
Shorten print job name on Cocoa. r=mstange
Backed out in https://hg.mozilla.org/integration/autoland/rev/7ec4ec57761e - at least on Windows PGO (the only thing that has gotten around to running and failing so far), you get https://treeherder.mozilla.org/logviewer.html#?job_id=136677694&repo=autoland
Not just Win PGO, it's actually busted on every opt build, I was just shocked by the fact that Win PGO was the fastest opt build to finish.
Hi philor,

(In reply to Phil Ringnalda (:philor) from comment #56)
> Not just Win PGO, it's actually busted on every opt build, I was just
> shocked by the fact that Win PGO was the fastest opt build to finish.

Thanks.
I'll fix this soon.
(In reply to Mantaroh Yoshinaga[:mantaroh] from comment #58)
> Comment on attachment 8913051 [details]
> Bug 1185236 - Add adjustPrintJobName in order to avoid IPP job name limit.
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/180132/diff/4-5/

I forgot to include the nsString.h, So opt build failed.
I added this header into PrintTarget.cpp:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=00e9f6658df265e38d752c8376db5fb72ea56828
Pushed by mantaroh@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/388a4fce5b4e
Add adjustPrintJobName in order to avoid IPP job name limit. r=karlt
https://hg.mozilla.org/integration/autoland/rev/f39bad8999fa
Shorten print job name when GTK version is older than 3.18.2. r=karlt
https://hg.mozilla.org/integration/autoland/rev/fb2b1aa5bb5e
Add AdjustPrintJobNameForIPP() wide version. r=karlt
https://hg.mozilla.org/integration/autoland/rev/672c5ee7b8e4
Shorten print job name on Cocoa. r=mstange
Assignee: nobody → mantaroh
Distributor ID: Ubuntu
Description: Ubuntu 16.04.5 LTS
Release: 16.04
Codename: xenial

Firefox 61.0.1, bug still here
(In reply to RoMan from comment #65)
> Distributor ID: Ubuntu
> Description: Ubuntu 16.04.5 LTS
> Release: 16.04
> Codename: xenial
> 
> Firefox 61.0.1, bug still here

On Ubuntu 16.04 GTK version is 3.18.9
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: