Bug 82784 - bitmap list in Area dialog must not have pre-selection, when called from a shape with background image
Summary: bitmap list in Area dialog must not have pre-selection, when called from a sh...
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: UI (show other bugs)
Version:
(earliest affected)
4.2.2.1 release
Hardware: All All
: high major
Assignee: Michael Stahl (CIB)
URL:
Whiteboard: target:5.0.0 target:4.3.7 target:4.4.3
Keywords: bibisected, bisected, regression
: 89253 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-08-18 21:39 UTC by Regina Henschel
Modified: 2016-11-01 19:29 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Regina Henschel 2014-08-18 21:39:33 UTC
1. Insert a picture (any .png) into Draw document.
2. Right-click > Convert > To Contour
That converts the picture to a polygon, where the former picture is set as background bitmap with setting "AutoFit".
3. Right-click > Area
4. Uncheck "AutoFit"
5. OK

Notice that the picture is gone and the background is blank.
Expected behavior: The shape has still this picture as background.

It worked in
Version: 4.2.0.0.alpha0+
Build ID: 8624906bc437b242ea0df17801bc77d0f273f32c
TinderBox: Win-x86@6, Branch:master, Time: 2013-07-05_04:07:10
and in
Version: 4.1.6.2
Build-ID: 40ff705089295be5be0aae9b15123f687c05b0a

It fails in
Version: 4.2.2.1
Build-ID: 3be8cda0bddd8e430d8cda1ebfd581265cca5a0f
and fails in current versions.

The reason, why it fails, is the fact, that the list of bitmaps in the Area dialog has now a pre-selected item. So when you click OK to finish the dialog, this item is used as new background. In former versions no item was pre-selected and therefore the existing background picture was used and only its position settings were changes. In contrast to lists in HTML this list of bitmaps does not allow to unselect an item with ctrl-click.
Comment 1 Buovjaga 2014-11-12 12:49:24 UTC
Reproduced.

Win 7 64-bit Version: 4.4.0.0.alpha2+
Build ID: b021b5983c62e266b82d9f0c5c6d8d8900553827
TinderBox: Win-x86@42, Branch:master, Time: 2014-11-12_00:19:18

Ubuntu 14.10 64-bit Version: 4.4.0.0.alpha2+
Build ID: 1c526c9ddda5d52f7a4db5655a4ec60b8c62835c
TinderBox: Linux-rpm_deb-x86_64@46-TDF-dbg, Branch:master, Time: 2014-11-11_23:20:41
Comment 2 tmacalp 2015-01-16 19:48:57 UTC
Damn, I just typed out a full bug report before finding this one!  I'll paste here, since it appears to be the same bug.  My steps for getting the object with a custom image background are a bit different, and not really relevant to this report, but I'll leave them in. :)

Since this is obviously a regression from a previously working behavior, I've upping the severity from medium to high.  I tested this under various 32bit/64bit linux distributions, so I'll change from being 32bit MS Windows only to hardware all/all.


Description:
As of LO 4.2.x, if you create a basic draw object and attempt to insert a picture with that shape selected, Draw will apply a tiled bitmap of that image to the selected draw object.  If you look at the object's Area properties, you'll notice that there is an applied bitmap to the object, but that bitmap doesn't show in the bitmap list.

In 4.2.0.4, this weird behavior actually kind of worked.  When you visit the Area dialog's "Area" tab, Fill would be set to "Bitmap" and none of the items in the bitmap list would be selected.  The preview at the bottom would show your custom image.  This allowed you to actually change attributes (enable autofit instead of tile, change offset, size, etc...).  And you could then click OK to apply.

At some point after 4.2.0.4, this behavior was broken, so that whenever you open the "Area" tab, the bitmap style "Blank" is defaulted to being selected.  There is no way to choose your custom temporary bitmap style in the dialog, since it doesn't appear in the list.  So any changes you make will also apply the new bitmap style of Blank.  So you end up with a White bitmapped shape.

Steps to reproduce:
1. Create new Drawing
2. Use rectangle tool to draw a rectangle
3. Insert -> Image from file
4. Select an image and click Open
5. Format -> Area

Expected:
The Area dialog should appear with our custom image in the preview window and nothing selected in the bitmap list.

Actual:
The Area dialog appears with just white showing in the preview window and "Blank" selected in the bitmap list.  Because of this, our custom bitmapped object will be forced to become white.

6. Click the "Tile" checkbox to disable "Tile" and enable "AutoFit"
7. Click OK

Expected:
Our object with a custom bitmap background should now be autofit to our object.

Actual:
Our object is now white.

Notes:
The most elegant way of handling this issue would be to actually to enhance the dialog to SHOW our temporary image-generated bitmap in the list of selectable bitmaps for this object.  We don't want to actually add this image to our permanent list of bitmaps, though.  Maybe in this case, we could have an item in the list titled "Applied bitmap," "Current bitmap," or something similar.

It would also be a nice enhancement if there was a way to choose a temporary image bitmap in the Area dialog without adding it to the permanent bitmap list.  That should probably be another enhancement request, though.

The easy fix is to simply revert to the old behavior of having nothing actually selected.

I've bibisected:

ce550d732fd43380870f273e6674894d5bb1cdf2 is the first bad commit
commit ce550d732fd43380870f273e6674894d5bb1cdf2
Author: Bjoern Michaelsen <bjoern.michaelsen@canonical.com>
Date:   Sun May 11 10:33:38 2014 +0000

    source-hash-b634aa656a74e1f8ebeaf8a9092829294c49171d
    
    commit b634aa656a74e1f8ebeaf8a9092829294c49171d
    Author:     Chris Sherlock <chris.sherlock79@gmail.com>
    AuthorDate: Fri Feb 7 22:50:28 2014 +1100
    Commit:     Caolán McNamara <caolanm@redhat.com>
    CommitDate: Fri Feb 7 20:16:23 2014 +0000
    
        Remove commented code in OutputDevice::ReMirror
    
        The following code was reimplemented in a cleaner fashion some time
        ago. However, it was just commented out and never removed. There is
        no need for it now, so removing. It's in the DVCS if it's really
        needed :-)
    
        Change-Id: Ia7d3c480ba70ccbd8dcf2808d9b712499c4cef4f
        Reviewed-on: https://gerrit.libreoffice.org/7913
        Reviewed-by: Caolán McNamara <caolanm@redhat.com>
        Tested-by: Caolán McNamara <caolanm@redhat.com>

:100644 100644 05874cf3764db72071b0d1b5d25bab8c7b5f7fb0 253114e5235f294b17b079bdbbc5aeeaae0e15b2 M      ccache.log
:100644 100644 b6d22eb23af685e0bf6f87ef3f5ef02a999ddc19 e1a110f455c5d638b61505bc8751b78557282e98 M      commitmsg
:100644 100644 a958005a2c4cd7a54b08bd25ec524930b3f5f0d3 bc63e7f72f8898a76d822c221a1fc5d1c2c91cf5 M      make.log
:040000 040000 fb4d7c258fc0eadfaeae60b239d3efd2c55e4e26 e2de3407c4f93021f77aae11d25f83026b900b1f M      opt


$ git bisect log
git bisect start 'last43onmaster' 'last42onmaster'
# bad: [4fcd68ce4979f85fda4568f4b419a4b41d07345f] source-hash-2c4621c87ed3a7b19de195c21494c9a381e72b2e
git bisect bad 4fcd68ce4979f85fda4568f4b419a4b41d07345f
# good: [0d4c20a601a3cfff27d6685d0e81463086bd9d74] source-hash-f1b1e73227471192682d303a58618ca8bd65a74d
git bisect good 0d4c20a601a3cfff27d6685d0e81463086bd9d74
# bad: [3c72d6d27e2a0c420f74941355400b0834c550bb] source-hash-c30677731c55688c764a669ecea1b1c4d17ae57d
git bisect bad 3c72d6d27e2a0c420f74941355400b0834c550bb
# good: [1f32fb58159d7f43a4bcb838765261d5274cbf38] source-hash-4a169e4203c10ec8f76b9bcb33882c82b65c7bab
git bisect good 1f32fb58159d7f43a4bcb838765261d5274cbf38
# good: [1da81d12775c421b7a04cd3a579fa555442c35f6] source-hash-ac01fd51822dc006abec578d61d66f7a169c19cb
git bisect good 1da81d12775c421b7a04cd3a579fa555442c35f6
# bad: [273a2f4e453564a9aad29b4e4fb0d3c46938bb9e] source-hash-863f1bfca71a5eb084931b49393fb7a9c5a0deaf
git bisect bad 273a2f4e453564a9aad29b4e4fb0d3c46938bb9e
# bad: [62e28acf6d832fb1ad030889541aad3f626612ba] source-hash-12e0102f39ee3a0398a4369bbc4af4ea0f51ca14
git bisect bad 62e28acf6d832fb1ad030889541aad3f626612ba
# bad: [ce550d732fd43380870f273e6674894d5bb1cdf2] source-hash-b634aa656a74e1f8ebeaf8a9092829294c49171d
git bisect bad ce550d732fd43380870f273e6674894d5bb1cdf2
# first bad commit: [ce550d732fd43380870f273e6674894d5bb1cdf2] source-hash-b634aa656a74e1f8ebeaf8a9092829294c49171d
Comment 3 Matthew Francis 2015-01-19 07:09:57 UTC
The behaviour seems to have changed as of the below commit.

Adding Cc: to mstahl@redhat.com. Could you possibly take a look at this? Thanks


commit 38d0047da7f964c862360b48d88cc869ad376b6b
Author: Michael Stahl <mstahl@redhat.com>
Date:   Fri Feb 7 18:25:39 2014 +0100

    related: fdo#74230: Area tab page: prevent default gradient/hatch/bitmap
    
    The Area dialog for shapes unfortunately makes it possible to set
    gradients/hatching/bitmap that uses the default items in the
    SfxItemPool.  These items however cannot be stored to ODF files, since
    these are represented as elements, not as attributes on a
    style:default-style; what we get for defaults is a somewhat silly
    draw:fill="hatch" without an draw:fill-hatch-name.
    So prevent the dialog from creating them by forcing a selection of an
    entry in the list.
    
    Change-Id: I67cc6dbbf7b491f06d094d4de1e9c3ffe79b01f5
Comment 4 Matthew Francis 2015-02-21 15:53:47 UTC
*** Bug 89253 has been marked as a duplicate of this bug. ***
Comment 5 tmacalp 2015-04-16 16:07:50 UTC
It appears that this bug affects more than just Draw.  As of LO 4.4, this bug affects ANY LO object that allows for setting the area to an image.

For instance, in Writer, this affects applying an image background to page, header, footer, paragraph, frame, table, etc...

This regression is really quite nasty.  Basically, once you've set an object's area to an image, the only way to open the object's properties dialog without losing that image is to somehow open the dialog without landing on the area tab.  Since the dialog remembers your last tab, I guess a work-around is to open the dialog, click another tab, cancel, and then reopen the dialog.  But that is a lot of extra work.

The more I think about it, the more I think merging the fill types of "Bitmap" with "Image" into a single tab is a mistake.  There may be a lot of overlap with their settings, but it's unintuitive, cludgy, and currently broken.

I'm upping the priority from Normal->Major because it is extremely difficult to set object properties from the dialog without losing data (your image).  It's also a regression that now affects multiple components.

This should probably be moved from Draw to something more generic, UI?
Comment 6 Michael Stahl (CIB) 2015-04-17 21:31:32 UTC
sorry, evidently the assumption that all "real" images would be entered into the list was wrong...

fixed on master
Comment 7 Commit Notification 2015-04-17 21:32:18 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=171fb61c6526daf83d6948a543a1614215590946

tdf#82784: cui: Area tab page: do not override imported bitmaps

It will be available in 5.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 8 Commit Notification 2015-04-20 09:00:00 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "libreoffice-4-4":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=e6530bd11bd37cc8bdec7f075c78d9ed52862424&h=libreoffice-4-4

tdf#82784: cui: Area tab page: do not override imported bitmaps

It will be available in 4.4.4.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 9 Commit Notification 2015-04-20 09:01:22 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "libreoffice-4-3":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=00dc0e2a2d505e5281a9f5a32fa658e5fca6a6ea&h=libreoffice-4-3

tdf#82784: cui: Area tab page: do not override imported bitmaps

It will be available in 4.3.8.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 10 Commit Notification 2015-04-20 14:34:50 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "libreoffice-4-3-7":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=cd16cf3e499f9d0f345cd52cb92ae945189ff480&h=libreoffice-4-3-7

tdf#82784: cui: Area tab page: do not override imported bitmaps

It will be available in 4.3.7.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 11 Commit Notification 2015-04-21 12:23:05 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "libreoffice-4-4-3":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=23f37ccf1bc8f02f5048aac62245dbc999806e1e&h=libreoffice-4-4-3

tdf#82784: cui: Area tab page: do not override imported bitmaps

It will be available in 4.4.3.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 12 Paddy Landau 2015-04-21 14:58:33 UTC
This appears to be fixed in the latest master 5.0.0.0.alpha1+.

Thank you.
Comment 13 Denis RADWAN 2015-04-23 11:31:31 UTC
fixed with :
Version: 4.3.8.0.0+
Build ID: 589f7cbe3789224a8836f3cd4705f067be6789c5
TinderBox: Win-x86@42, Branch:libreoffice-4-3, Time: 2015-04-20_12:02:51

and
Version: 4.4.4.0.0+
Build ID: d0e96a62a1f497020f5d15640af05ed6224f98e0
TinderBox: Win-x86@51-TDF, Branch:libreoffice-4-4, Time: 2015-04-21_20:22:01
Locale: fr_FR

Nice work !
Thank you ;)
Comment 14 Buovjaga 2015-04-23 11:40:13 UTC
Let's set to VERIFIED to celebrate.
Comment 15 Robinson Tryon (qubit) 2015-12-17 08:30:20 UTC Comment hidden (obsolete)