Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[api-minor] Fix various issues related to pageSize, and display the size for the active page in the document properties dialog #9577

Merged
merged 4 commits into from
Mar 18, 2018
Merged

[api-minor] Fix various issues related to pageSize, and display the size for the active page in the document properties dialog #9577

merged 4 commits into from
Mar 18, 2018

Commits on Mar 18, 2018

  1. [api-minor] Fix various issues related to the pageSize information 

    The `getPageSizeInches` method was implemented on `PDFDocumentProxy`, which seems conceptually wrong since the size property isn't global to the document but rather specific to each page. Hence the method is moved into `PDFPageProxy`, as `get pageSizeInches` instead to address this.

Despite the fact that new API functionality was implemented, no unit-tests were added. To prevent issues later on, we should *always* ensure that new functionality has at least some test-coverage; something that this patch also takes care of.

The new `PDFDocumentProperties._parsePageSize` method seemed unnecessary convoluted. Furthermore, in the "no data provided"-case it even returned incorrect data (an array, rather than the expected object).
Finally, the fallback strings didn't actually agree with the `en-US` locale. This inconsistency doesn't look too great, and it's thus addressed here as well.
    @Snuffleupagus
    Snuffleupagus committed Mar 18, 2018
    Configuration menu
    Copy the full SHA
    e0ae157 View commit details
    Browse the repository at this point in the history

  2. Tweak the pageSize l10n strings for the document properties dialog 

    The units are currently repeated after each dimension, which seems unnecessary and is also not done in other PDF viewers (such as e.g. Adobe Reader).

Furthermore, the name of the l10n arguments can be simplified slightly, since the name of the strings themselves should be enough information.

Finally, the `width`/`height` should be formatted according to the current locale, as is already done for other strings in the document properties dialog.
    @Snuffleupagus
    Snuffleupagus committed Mar 18, 2018
    Configuration menu
    Copy the full SHA
    1730447 View commit details
    Browse the repository at this point in the history

  3. Don't unnecessarily update the fileSize, in the document properties d… 

    …ialog, when it's already been correctly set
    @Snuffleupagus
    Snuffleupagus committed Mar 18, 2018
    Configuration menu
    Copy the full SHA
    adeaefe View commit details
    Browse the repository at this point in the history

  4. Display the pageSize of the *currently* active page in the document p… 

    …roperties dialog
    @Snuffleupagus
    Snuffleupagus committed Mar 18, 2018
    Configuration menu
    Copy the full SHA
    51ddcd6 View commit details
    Browse the repository at this point in the history