hplip-3.18.5 - pmlfax.py - revert the change

[zdohnal]

Hi,

you made a change in fax/pmlfax.py in the latest release - removed encoding to utf-8 between lines 144-147 and 156-159. These changes results in that string data type is sent to utils.printable method, but there is still used translate method for binary string -> that makes f.e. hp-setup to fail with traceback.

I reverted the change in attached patch - would you mind adding it to hplip project? Or fully convert the relevant code to strings?

[srinivas (srinivas5)]

Hi zdohnal,

Sorry for the delay in the response and thanks for updating your comments on the fix.
I have tested the change after the fix made and i have not seen any traceback error.
Are you seeing the traceback error after this fix?
If yes, can you please share screenshot of the error so that i can look into it.

Thanks,
Srinivas Teja.

[zdohnal (zdohnal)]

Hi Srinivas,

I think you misunderstood me. I created the attached patch and tested it, so I know it fixes the traceback issue (btw the change in base/utils.py needs to be reverted too to have fax setup working again, so the patch is incomplete).
What I asked for is if you mind adding my patch (which is attached to this bug report) to the project to have it fixed.

[srinivas (srinivas5)]

Hi zdohnal,

Iam expecting you are asking me to revert the below change in utils.py along with adding the patch attached.
###########################################
def printable(s):
    if s:
        return s.translate(identity, unprintable)
    else:
        return ""
to

def printable(s):
    return s.translate(identity, unprintable)
###########################################
I have tried this earlier but still there was traceback error and so the following changes were made.
So please check if you see traceback error or fax issue with the fix made.
Kindly confirm back.

Thanks,
Srinivas Teja.

[zdohnal (zdohnal)]

Without reverting this change:

####################################
+ if s:
+ return s.translate(identity, unprintable)
+ else:
+ return ""
####################################

back to:

####################################
     return s.translate(identity, unprintable)
####################################

the 'hp-setup --debug' command ends with trackback:

Traceback (most recent call last):
  File "/usr/share/hplip/ui5/setupdialog.py", line 1305, in NextButton_clicked
    self.showAddPrinterPage()
  File "/usr/share/hplip/ui5/setupdialog.py", line 729, in showAddPrinterPage
    self.readwriteFaxInformation()
  File "/usr/share/hplip/ui5/setupdialog.py", line 1128, in readwriteFaxInformation
    self.fax_number = to_unicode(d.getPhoneNum())
  File "/usr/share/hplip/fax/pmlfax.py", line 147, in getPhoneNum
    return data.decode('utf-8')
AttributeError: 'str' object has no attribute 'decode'

Only if I revert both changes - these in fax/pmlfax.py and base/utils.py - I get past the tracebacks.

[zdohnal (zdohnal)]

See updated patch.

[srinivas (srinivas5)]

Hi zdohnal,

In the latest release, we did not see any traceback error during hp-setup.
Can you please share the screenshot of the error without making any changes.
Also share printer model and the distro details so that we can check.

Thanks,
Srinivas Teja.

[zdohnal (zdohnal)]

Required screenshot - printer model, python3 version and OS version (Fedora 29) are there too. hp-setup ends with the same traceback on Fedora 28 too (when I don't apply my patch).

[srinivas (srinivas5)]

HI zdohnal,

We are yet to support Fedora 28. Latest Fedora version we support is Fedora 27.
So can you please verify on Fedora 27 and confirm if you are seeing the same traceback issue.

Note: Kindly check without applying your patch.

Thanks,
Srinivas Teja.

[zdohnal (zdohnal)]

Result is the same for F27 too.

$ rpm -q python3
python3-3.6.5-1.fc27.x86_64

[Martin Wilck (mwilck)]

Srinivas:

The result of printable() needs to be a bytes object in python3, otherwise x.decode() fails for obvious reasons. So, if you don't like the patch reverting the if clause (the patch is fine, AFAICS),
you should at least change printable() to this:

def printable(s):
    if s:
        return s.translate(identity, unprintable)
    else:
        return b""

[Martin Wilck (mwilck)]

This bug is still present in hplip 3.19.12.

The patch from comment 1 should be applied. And the change from comment 12. Or better:

diff --git a/base/utils.py b/base/utils.py
index f268cf8..cc77820 100644
--- a/base/utils.py
+++ b/base/utils.py
@@ -954,10 +954,7 @@ else:

 def printable(s):
- if s:
- return s.translate(identity, unprintable)
- else:
- return ""
+ return s.translate(identity, unprintable)