'm' units identifier not handled correctly

Bug #1395637 reported by Tavmjong Bah on 2014-11-24
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Inkscape
High
Tavmjong Bah

Bug Description

The 'm' units identifier along with 'ft' are not part of SVG and need special handling. The special handling for 'ft' seems to work but that for 'm' can lead to an incorrect 'viewBox'.

To reproduce:

1. Open an Inkscape with the default SVG file.
2. Open the XML dialog and observe the 'viewBox' attribute.
3. Open the Document Properties dialog to the 'Page' tab.
4. Change the Units in 'Custom Page Size' to 'm'
5. Change the Units back to the original units ('mm' in the case of trunk).

The 'viewBox' values will bo 100 times bigger than they should be.

Analysis:

The file units.xml should contain all the information needed to add new units to Inkscape. However, as 'm' and 'ft' are not part of SVG special handling is required. This special handling happens in two places: units.cpp for 'ft' and document.cpp for 'm' (setWidth(), setHeight()).

The need for special handling should be eliminated. As a quick fix, the special handling for 'm' should be moved to 'units.cpp'.

su_v (suv-lp) wrote :

See also comments for bug #1268355 wrt 'ft' and 'm' not part of SVG.

tags: added: units viewbox
su_v (suv-lp) wrote :

Reproduced with trunk revision >= 13632 on OS X 10.7.5

Changed in inkscape:
importance: Undecided → High
status: New → Confirmed
su_v (suv-lp) wrote :

AFAICT does not affect 0.91.x (based on the 'steps to reproduce').

Changed in inkscape:
milestone: none → 0.92
Tavmjong Bah (tavmjong-free) wrote :

Actually, 'ft' also occurs in svg-length.h and svg-length.cpp.

Tavmjong Bah (tavmjong-free) wrote :

#3: That is a good clue, as I had patched trunk with 13632 from trunk.

Alvin Penner (apenner) wrote :

fix committed to rev 13753
this fixes a bug in rev 13632

Changed in inkscape:
status: Confirmed → Fix Committed
su_v (suv-lp) on 2014-11-25
Changed in inkscape:
assignee: nobody → Alvin Penner (apenner)
milestone: 0.92 → none
status: Fix Committed → Fix Released
Tavmjong Bah (tavmjong-free) wrote :

Alvin, thanks for your prompt fix.

I have just checked in what I think is a better fix. That is moving the 'm' handling code from document.cpp to

  svg/svg-length.cpp
  svg/svg-length.h
  util/units.cpp

This is a more generic solution that follows the pattern from 'ft'.

See r13758

su_v (suv-lp) wrote :

@Tav - should this get backporting to the stable 0.91.x branch?

Changed in inkscape:
assignee: Alvin Penner (apenner) → Tavmjong Bah (tavmjong-free)
milestone: none → 0.92
status: Fix Released → Fix Committed
Tavmjong Bah (tavmjong-free) wrote :

@~suv: Probably yes, along with Alvin's accuracy fix.

su_v (suv-lp) on 2014-11-26
tags: added: backport-proposed
ScislaC (scislac) wrote :

backported trunk commits r13753 & r13758 to 0.91.x r13661

tags: removed: backport-proposed
Changed in inkscape:
milestone: 0.92 → 0.91
Bryce Harrington (bryce) on 2015-02-23
Changed in inkscape:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers