Download it now!
Bug 103602 - new documents fail ODF validation with Error: unexpected attribute "draw:fill"
Summary: new documents fail ODF validation with Error: unexpected attribute "draw:fill"
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
5.3.0.0.alpha0+
Hardware: All All
: high major
Assignee: Not Assigned
URL:
Whiteboard: odf target:7.0.0 target:6.4.5 target:...
Keywords: bibisected, bisected, regression
Depends on:
Blocks: 72238 Draw-Cell-Filling Area-Fill-Tab ODF-export-invalid Whole-Page-Filling
  Show dependency treegraph
 
Reported: 2016-10-31 14:04 UTC by Justin L
Modified: 2020-08-22 07:09 UTC (History)
16 users (show)

See Also:
Crash report or crash signature:


Attachments
Test doc (before change) with illegal fill-related attributes (18.29 KB, application/vnd.oasis.opendocument.text)
2017-07-30 03:04 UTC, Luke Deller
Details
Test doc (after proposed change) using loext: namespace (18.29 KB, application/vnd.oasis.opendocument.text)
2017-07-30 03:07 UTC, Luke Deller
Details
Manually applied the suggested changes (12.53 KB, application/vnd.oasis.opendocument.text)
2017-08-17 08:19 UTC, Regina Henschel
Details
An analysis and some versions of schema changes (4.10 KB, application/zip)
2017-08-30 23:33 UTC, Regina Henschel
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Justin L 2016-10-31 14:04:28 UTC
Description:
New documents do not create a valid ODF 1.2 extended file if the properties dialog is opened (for sure with the page properties, likely any property that contains the fill area tab).

Steps to Reproduce:
1. Start a new Writer document 
2. Open the Page Properties (Format - Page) and press OK to close it.
3. Save as .odt format
4. Check validity at https://odf-validator.rhcloud.com/ (using 1.2 extended, not auto-detect)

Actual Results:  
The document is NOT extended conformant ODF1.2!

Expected Results:
The document is extended conformant ODF1.2!


Reproducible: Always

User Profile Reset: 

Additional Info:
Bibisected with Linux daily debug 5.3.
It broke somewhere between:
2016-10-11 update credits Christian Lohmaier 	29fe57afd79c96c5ff251c2468ababda5e506564

and 

2016-10-12 ADO: return correct css::sdbcx::CompareBookmark values
Stephan Bergmann 66b67f40a7785f08ae214e62b669e001148b474c

likely by either
[GSoC] Remove fill style tabs from area fill dialog 	d19ec9a9bd371248afd7c3ca091a54b1782fddf7

or

[GSoC] Move all fill style tabs inside area tab 686349476e03f951f4a9ff9755b9f71951b64ea5


User-Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:49.0) Gecko/20100101 Firefox/49.0
Comment 1 Aron Budea 2016-10-31 15:12:04 UTC
It's the second candidate:
https://cgit.freedesktop.org/libreoffice/core/commit/?id=686349476e03f951f4a9ff9755b9f71951b64ea5

"[GSoC] Move all fill style tabs inside area tab"

Full bibisect log:
# bad: [845539f0cf09355a7baa6d1cfbf6ad478a983da5] source sha:8dc495c93239739629683bb29e4110f5c57c94f0
# good: [33e60eae04c889baf52713a73dc9944015408914] source sha:5b168b3fa568e48e795234dc5fa454bf24c9805e
git bisect start '845539f0cf09355a7baa6d1cfbf6ad478a983da5' '33e60eae04c889baf52713a73dc9944015408914'
# good: [9e0af0bfb61955f2a7bcae3a25a403cfc01919f4] source sha:e8365711e817876ee45b282fc16977b55f4dbca8
git bisect good 9e0af0bfb61955f2a7bcae3a25a403cfc01919f4
# good: [45cf7c7435abbc97ae9773953e5f582784b91c1c] source sha:d5f1e7c5adac5f38379c56b536640f3aaf9ec7d5
git bisect good 45cf7c7435abbc97ae9773953e5f582784b91c1c
# good: [be8f9784b28d086ec451f86ed968fe0364b26382] source sha:8413d6a91d5d03c0c47a8fbd2e41cd45cec95a1c
git bisect good be8f9784b28d086ec451f86ed968fe0364b26382
# skip: [681fc5fe4f850e7885faf8fc43b1c7f377f6f902] source sha:613213240a84327a0a72bb5ee98cc88d50c1fd0e
git bisect skip 681fc5fe4f850e7885faf8fc43b1c7f377f6f902
# bad: [6892f3a21a61c0aeb726e060576b291e08b052d6] source sha:398d641664baa6eaeb34789f0aebfd21e73edef3
git bisect bad 6892f3a21a61c0aeb726e060576b291e08b052d6
# bad: [c8e7036b5e5248aa5b20315767d55832fc7d0375] source sha:0f613adbfa44fb92e84e73a3fa7ea050c072944c
git bisect bad c8e7036b5e5248aa5b20315767d55832fc7d0375
# good: [1c9e9a7ab9725516a6f9f20326439a6c025cd222] source sha:583ac140dcf6f11b21bc9c52516ba2bbcbd37a95
git bisect good 1c9e9a7ab9725516a6f9f20326439a6c025cd222
# bad: [3177d9338fb3d48f40a5a13b980d6e829f4bf76d] source sha:e6a05e9f3276cccce5d72adce24a8d2fee6b8b7b
git bisect bad 3177d9338fb3d48f40a5a13b980d6e829f4bf76d
# good: [459a28dbbb27faacb725b523cb25555671b2debd] source sha:34698034dbfe59bf444709fcf54d01f8a6f03f83
git bisect good 459a28dbbb27faacb725b523cb25555671b2debd
# bad: [281ec4b684aeba7aaf527e64e6cb187b8a1e95d5] source sha:a77ff8446e7ed2255b11406b4ccce7ec742d7c67
git bisect bad 281ec4b684aeba7aaf527e64e6cb187b8a1e95d5
# bad: [cff3c3864a46cefde1b6e3145995b28ae6f9f008] source sha:0d93900801224b797741e9a1abf305109fa35665
git bisect bad cff3c3864a46cefde1b6e3145995b28ae6f9f008
# good: [9c0485b505f97777dbb84accf1a902a234b234f5] source sha:ed3a4fcc878e8f3ce720520d60448bc962da0754
git bisect good 9c0485b505f97777dbb84accf1a902a234b234f5
# bad: [b3691ee4fdddd35b2dcea8bec7daecc981792844] source sha:686349476e03f951f4a9ff9755b9f71951b64ea5
git bisect bad b3691ee4fdddd35b2dcea8bec7daecc981792844
# good: [2d0914fa553e754fe5942c037898e21102d93388] source sha:da01e9ec5dfb7787b4a3669486b3940590933850
git bisect good 2d0914fa553e754fe5942c037898e21102d93388
# first bad commit: [b3691ee4fdddd35b2dcea8bec7daecc981792844] source sha:686349476e03f951f4a9ff9755b9f71951b64ea5
Comment 2 Regina Henschel 2016-10-31 16:08:23 UTC
It is a hard problem. The "draw:fill" attribute may not be used in the element <style:page-layout-properties>. And the element <style:page-layout> has no other children or attributes that are suitable.

For Draw and Impress, this is solved by an element <style:style> of style family "drawing-page". But this family is only usable with draw and presentation documents.

For Calc it is solved by restricting the background filling to color and image, because those do not need an attribute "draw:fill". Color is written by the attribute "fo:background-color" and an image by the child element <style:background-image> of the element <style:page-layout-properties>.
Writer was restricted in the same way in older versions of LibreOffice.

So currently exists no way to express other kind of fillings beside color or image for a text document or for a spreadsheet in ODF.

There are suggestions required, how to solve this problem in ODF. Such considerations would have been necessary _before_ implementing the new feature.

Another problem is, to get it into the namespace "loext", because only whole elements and attributes have a namespace, but not a single value.

Where should this be discussed, so that it is really public? Here in the issue, or on the dev mailing list, or in a Wiki page or ...?
Comment 3 Katarina Behrens (Inactive) 2016-11-01 15:32:14 UTC
Can anyone please 'splain what exactly the issue here is, pretty please? For example, paste some ODF XML diff before vs. after

> So currently exists no way to express other kind of fillings beside color or
> image for a text document or for a spreadsheet in ODF.

As far as I can see, Format > Page dialog in Writer (Libreoffice 5.1, so the old one existing prior to GSoC code merge) offers *all* kinds of fills for a page, not only colour and bitmap/image. Does this mean that the other fill types have never been saved to ODF and now they are?

As for Calc (again in Libreoffice 5.1), I only see the option to set background to solid colour (Format > Cells > Background) and the tab page used for that is different than the tab page used for shape fill (Background vs. Area, they only look similar)
Comment 4 Regina Henschel 2016-11-01 16:51:16 UTC
(In reply to Katarina Behrens (CIB) from comment #3)
> As far as I can see, Format > Page dialog in Writer (Libreoffice 5.1, so the
> old one existing prior to GSoC code merge) offers *all* kinds of fills for a
> page, not only colour and bitmap/image. Does this mean that the other fill
> types have never been saved to ODF and now they are?

The file format error slipped in from LO 4 to LO 5.
Version 4.1 and 4.2 had the old background selection with color and image as still available in Calc.
Version 4.3 and 4.4 had the item "gradient" added, but that was without any function.
Version 4.5 had got fill kind "gradient" and "hatching" and the worked, but they were not written to file, and therefore any gradient or hatching was lost on save and replaced with a single color fill in LO 4.5.

Writing invalid ODF slipped in after version 4.5 and it is not a problem of this year GSoC.

The error itself was made in version 4, when adding a new kind of filling, without considering ODF file format.
Comment 5 Xisco Faulí 2016-11-03 21:39:00 UTC
Adding Cc: to Rishabh Kumar
Comment 6 Luke Deller 2017-07-24 13:01:35 UTC
I understand from these comments that ODF does not support page fill by gradient/hatching for text documents, so we will need to move this information into the "loext" extension namespace.

To achieve this how about we make a child element of <style:page-layout-properties> named <loext:page-fill>, and move all the draw:fill-* attributes onto this child element?  Or any better suggestions?
Comment 7 Regina Henschel 2017-07-24 15:48:42 UTC
Please have look at the similar issue bug 94768.
Comment 8 Regina Henschel 2017-07-24 21:33:02 UTC
Thorsten, can you please have a look at it, how a solution should look in principle?

(In reply to Luke Deller from comment #6)
> I understand from these comments that ODF does not support page fill by
> gradient/hatching for text documents, so we will need to move this
> information into the "loext" extension namespace.
> 
> To achieve this how about we make a child element of
> <style:page-layout-properties> named <loext:page-fill>, and move all the
> draw:fill-* attributes onto this child element?  Or any better suggestions?

I'm no sure about a solution. We need to keep in mind, how a final solution without namespace "loext:" can be realized in ODF 1.3. Do you have an idea about changing the schema and the specification?

From a technical point of view, you can put all the currently not allowed draw:fill-* attributes individually into the "loext:" namespace, without a new element. Please talk to Thorsten Behrens about a principle solution, perhaps via IRC.
Comment 9 Thorsten Behrens (CIB) 2017-07-24 22:14:30 UTC
(In reply to Regina Henschel from comment #8)
> Thorsten, can you please have a look at it, how a solution should look in
> principle?
> 
No real better ideas - but loext:page-fill seems redundant to me, too. Perhaps we  can restructure the schema a bit, so that fill attributes are grouped better, instead of repeatedly listed at various places?
Comment 10 Regina Henschel 2017-07-25 16:37:31 UTC
All the draw:fill-* attributes are in a <define name="style-graphic-fill-properties-attlist"> symbol in the schema. Unfortunately it has some attributes more than needed here. We could add a reference to the "style-graphic-fill-properties-attlist" symbol into the <define name="style-page-layout-properties-attlist"> symbol in the schema. I'm not sure, whether we need to exclude some of the attributes in the spec description and how this has to be done. Perhaps we need to extract some attributes from the <define name="style-graphic-fill-properties-attlist"> and use them instead.

The fo:background-color attribute is wrapped in a <define name="common-background-color-attlist"> symbol in the schema.

I prefer to have the draw:fill-* attributes at the same level as the fo:* attributes. ODF1.2 has already a rule how to handle draw:fill and fo:background-color, if both attributes exist on the same style, in the section 20.175.

So here it would mean to exchange the erroneous "draw:fill" with a "loext:fill", same for all other attributes on which the validator reports an error. All kind of fill styles have to be checked.

I know, that for paragraph background the way with new <loext:graphic-properties> was chosen. But the paragraph style has a different context. It is a <style:style> element and the selection of the attributes is done by family, whereas the page background belongs to a <style:page-layout> element.

Thorsten: Is it OK for you?

Luke: Do you want to work on this issue? If yes, then assign it to you. And if you have finished work on the issue, please add the new use of "loext:" namespace to https://wiki.documentfoundation.org/Development/ODF_Implementer_Notes/List_of_LibreOffice_ODF_Extensions
Comment 11 Luke Deller 2017-07-30 03:04:07 UTC
Created attachment 134987 [details]
Test doc (before change) with illegal fill-related attributes

Attached is a test document (before change) with various background fills on the page, header and footer.  These fill-related properties are saved as invalid attributes on the elements <style:page-layout-properties> and <style:header-footer-properties>
Comment 12 Luke Deller 2017-07-30 03:07:03 UTC
Created attachment 134988 [details]
Test doc (after proposed change) using loext: namespace

Here is the same test document, but using attributes on the loext: namespace as proposed.

Looks good?
Comment 13 Regina Henschel 2017-07-30 12:23:47 UTC
The file with loext: namespace looks good. Please test in addition, that the loext: namespace is not written in case, you set the save option to "1.2" instead of "1.2 Extended (recommended)" in Tools > Options > Load/Save. It should be correct automatically, but to be sure.
Comment 14 Regina Henschel 2017-08-16 14:03:30 UTC
Putting the needed attributes somehow and without plan into a private namespace, would make the documents validate against "ODF 1.2. extended". But such solution looks "quick&dirty" to me. A concept is needed how the specification for pages in Writer and Calc has to be changed. For example a solution for page-wide background images is effected by such concept too. So please develop such concept _before_ implementing a solution.
Comment 15 Regina Henschel 2017-08-17 08:19:22 UTC
Created attachment 135608 [details]
Manually applied the suggested changes

I suggest a different approach:
Add the attribute draw:style-name to the element <style:master-page> and let is refer a <style:style> element with style:family="drawing-page".

This family allows all the needed attributes.

Advantage:
(1) No private namespace needed. The document validates against the ODF1.2 schema.
(2) The style family "drawing-page" provides the attribute draw:background-size. With its value "full" it would be possible implement the long demanded background fill till page edge without any change to ODF1.2.

It might be needed to make some additions to the textual part in the ODF1.2 specification in "19.219.27<style:master-page>" to explicitly allow this for text documents and spreadsheets too.
Comment 16 Luke Deller 2017-08-28 12:48:10 UTC
Reusing style:family="drawing-page" certainly does sound more attractive than using a private namespace.

What would we do with the header and footer backgrounds, currently represented as attributes on <style:header-footer-properties>?  (See attachment 134987 [details] for an example of this).
Comment 17 Regina Henschel 2017-08-28 14:07:48 UTC
I have discussed this with Thorsten. He too thinks, that for the page properties we should use a <style:style> element of family "drawing-page" and the reference in the draw:style-name attribute.

For header and footer exists no solution with an existing element. Similar filling problems exist with paragraph, section, table, table-row and table-cell. They all have the attribute fo:background-color and the child <style:background-image> but no complex fillings with gradient, hatch or transparency. Therefore I currently investigate, whether a common solution is possible. I'm working on a solution for the schema, but I have not finished yet. There is my mail https://lists.freedesktop.org/archives/libreoffice/2017-August/078296.html. In case you have ideas, we can discuss it there.

So if you want to start, I suggest to first handle those parts, which can be solved in a "drawing-page". That is independent from the header-footer-properties. Unfortunately it is more complex than adding a namespace. Are you interested nevertheless?
Comment 18 Luke Deller 2017-08-29 13:17:43 UTC
Okay that sounds like a good plan.  Yes it does look more complex because Writer does not currently understand this style family, but I am reading through the code for importing this into Draw and will have a look into how this might be refactored to be used in Writer also.
Comment 19 Regina Henschel 2017-08-30 23:33:06 UTC
Created attachment 135889 [details]
An analysis and some versions of schema changes

I have examined, how changes of the schema might work. There is no decision yet, but I want to inform you about the current state.
Comment 20 Luke Deller 2017-08-31 13:24:40 UTC
Thank you for sharing that Regina.

Regarding variants A & B, and how LibreOffice can write valid documents before ODF 1.3: would we just emit these attributes in the "loext:" namespace, otherwise the same as what you have described, until ODF 1.3?  I appreciate that having so many attributes in the extension namespace is not particularly nice.

Regarding the issues with using <style:graphic-properties> that you mention, do you think we have a similar situation with <style:drawing-page-properties> already?  I am thinking of the attributes it can have which are intended only for presentation documents, such as smil:type for page transition effects.  I see some constraint suggested by the last sentence of section 16.39:

> Within presentation documents, the draw page style may contain presentation
> formatting properties.

(This seems a bit loose - it does not define precisely what "presentation formatting properties" means, nor does it explicitly specify the inverse: that non-presentation documents should not contain these properties)
Comment 21 Regina Henschel 2017-08-31 15:51:28 UTC

(In reply to Luke Deller from comment #20)
> Regarding variants A & B, and how LibreOffice can write valid documents
> before ODF 1.3: would we just emit these attributes in the "loext:"
> namespace, otherwise the same as what you have described, until ODF 1.3?  I
> appreciate that having so many attributes in the extension namespace is not
> particularly nice.


Yes putting attributes into loext namespace works for validators. Although the algorithm in http://docs.oasis-open.org/office/v1.2/os/OpenDocument-v1.2-os-part1.html#ForeignElementsAndAttributes does not explicitly mention attributes. (add this to my ToDo-List for ODF TC) So technical version B would be possible too.

Nevertheless it would generate a conflict for drawing and presentation documents having the same attributes in <style:page-layout-properties> and <style:drawing-page-properties> in case "style-graphic-fill-properties-attlist" would be add to <style:page-layout-properties> in the schema. Therefore I have suggested to allow <style:drawing-page-properties> for text documents and spreadsheets. That gives a unique place for page background formatting. Therefore I suggest to use <style:drawing-page-properties> also in Version B.

> 
> Regarding the issues with using <style:graphic-properties> that you mention,
> do you think we have a similar situation with
> <style:drawing-page-properties> already?

I think not, because <style:drawing-page-properties> only includes "style-graphic-fill-properties-attlist", so no conflict with margin/border/padding settings.

  I am thinking of the attributes it
> can have which are intended only for presentation documents, such as
> smil:type for page transition effects.  I see some constraint suggested by
> the last sentence of section 16.39:
> 
> > Within presentation documents, the draw page style may contain presentation
> > formatting properties.
> 
> (This seems a bit loose - it does not define precisely what "presentation
> formatting properties" means, nor does it explicitly specify the inverse:
> that non-presentation documents should not contain these properties)

And it does not explicitly specify, that these element cannot be used for text documents and spreadsheet documents. When the wording is improved, that can be added in the same run. You are right, that the description in section 16.39 needs improvement, perhaps something like "attributes in presentation and smil namespace are only applicable to presentation documents".



I see, that I have named the diffs different from my analysis :(

So to be clear, the difference is whether to include characters for gradient and hatch background fill, and whether to use a new element wrapping the attributes or use the attributes directly.

And I notice that using attributes directly in paragraph-properties and text-properties gives a problem, because the <style:style> element of family paragraph contains both paragraph-properties and text-properties. 

So perhaps better wrap the attributes as in version C/D ?
Comment 22 Michael Stahl (CIB) 2017-09-08 14:49:32 UTC
there is already bug 94768 about the draw:fill attributes,
where i've fixed it to use the "loext" namespace,
but probably these are additional cases which i didn't notice back then.
Comment 23 Regina Henschel 2017-10-15 16:10:58 UTC
I have written https://issues.oasis-open.org/browse/OFFICE-3937. It contains the proposal to allow <style:drawing-page-properties> for all kind of documents. If the TC agrees, the page fill properties can be written to a <style:style> element of family drawing-page, which is then referenced in the master style be a draw:style-name attribute. Because such would not violate the schema, an immediate implementation without loext namespace would be possible.
Comment 24 Armin Le Grand 2017-10-18 07:34:33 UTC
Hi Regina, thanks!
Comment 25 Regina Henschel 2017-10-24 09:37:10 UTC
The ODF TC has agreed to the proposal, so it is now possible to use <style:drawing-page-properties> to store the information for background filling in Writer and Calc too in a way, that conforms to the schema.
Comment 26 Xisco Faulí 2018-01-23 09:04:41 UTC
Dear Luke Deller,
This bug has been in ASSIGNED status for more than 3 months without any
activity. Resetting it to NEW.
Please assigned it back to yourself if you're still working on this.
Comment 27 QA Administrators 2019-01-29 03:48:09 UTC Comment hidden (obsolete)
Comment 28 Regina Henschel 2019-01-29 12:44:41 UTC
The problem still exists in Version: 6.3.0.0.alpha0+ (x64)
Build ID: 4c8349f138f1269120180c663d82b534d60c1f73
CPU threads: 8; OS: Windows 10.0; UI render: default; VCL: win; 
Locale: de-DE (en_US); UI-Language: en-US
Calc: threaded
Comment 29 Michael Stahl (CIB) 2020-05-05 10:59:20 UTC
it looks like only Writer supports the draw:fill properties on page styles,
while Calc only writes the old color/bitmap markup.

i'm afraid we'll have to continue to export the draw:fill on the style:page-layout-properties for now (at least in extended) because there's no import yet for drawing-page-properties/draw-page style.
Comment 30 Commit Notification 2020-05-19 08:28:02 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/4e98ba4ba5c17ab8ae1170662af645b9d2bfde84

tdf#103602 xmloff,sw: ODF 1.3 import: PageStyle with drawing-page style

It will be available in 7.0.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 31 Commit Notification 2020-05-19 08:29:23 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/97e9b250426ff09bba4546835ee9138d91e03fbb

tdf#103602 xmloff,sw: ODF 1.3 export: PageStyle with drawing-page style

It will be available in 7.0.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 32 Michael Stahl (CIB) 2020-05-20 11:13:38 UTC
the part that is possible to fix in ODF 1.3 via OFFICE-3937 is fixed on master now.

what remains is the general case of draw:fill on things that aren't master-pages; that can't be fixed until we get new features in ODF 1.4.
Comment 33 Commit Notification 2020-05-23 14:31:27 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "libreoffice-6-4":

https://git.libreoffice.org/core/commit/ca9a719169fa444b5d82942ae1d5600fa0114002

tdf#103602 xmloff,sw: ODF 1.3 import: PageStyle with drawing-page style

It will be available in 6.4.5.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 34 Regina Henschel 2020-05-27 15:21:29 UTC
I think, that the attribute drawing:background-size should be written in addition to the <style:drawing-page-properties> element. So when reading this element is implemented, the documents would have already the correct settings. That attribute is optional but has no default. So it would be good, if the documents contain the correct setting.

In Writer fill with gradient, color, pattern and hatch fills the entire page, so that would be drawing:background-size="full". Fill with bitmap still fills only inside border, so that would be drawing:background-size="border".



And I think, in case of saving to "ODF 1.3 strict", the attribute draw:fill and connected attributes should not be written to the <draw:page-layout-properties> at all. Users who need the additional features which are possible with attribute draw:fill and the connected attributes, can save to "ODF 1.3 extended". With such restriction in save to "ODF 1.3 strict" LibreOffice would still not render conform to the spec text (= draw:fill in a style disables <style:background-image> and <fo:background-color>), but at least the produced documents would contain valid XML.
Comment 35 Regina Henschel 2020-05-27 18:38:35 UTC
(In reply to Regina Henschel from comment #34)
With such restriction in save to "ODF 1.3
> strict" LibreOffice would still not render conform to the spec text (=
> draw:fill in a style disables <style:background-image> and
> <fo:background-color>), but at least the produced documents would contain
> valid XML.

I was wrong in this regard. The draw:fill attributes are used, and neither <style:background-image> nor <fo:background-color>. That works in LO 6.4.5 with ODF 1.3 format too.


But saving to "ODF 1.2 strict" writes the draw:fill attributes into <style:page-layout-properties> too. That should not happen for the "strict" file formats. Only in "ODF 1.2 extended" and "ODF 1.2 compatible" the faulty attributes should be written. Not using loext for "ODF 1.2 extended" is already a concession to older LibreOffice versions, so "ODF 1.2 strict" should be correct.
Comment 36 Commit Notification 2020-05-29 08:54:56 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/415fac828c4bb45fa23f7d81e93992b711f37810

tdf#103602 xmloff: ODF export: draw:background-size attribute

It will be available in 7.1.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 37 Commit Notification 2020-05-29 10:21:35 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/ef2e609b9c3c4b568e1d04360f81b5f614fa6bd8

tdf#103602 xmloff: ODF export of page style: don't export draw:fill

It will be available in 7.1.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 38 Commit Notification 2020-05-29 10:23:20 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "libreoffice-7-0":

https://git.libreoffice.org/core/commit/4166cf43d2a56b0c2e9a3bf4c15d906be025ada6

tdf#103602 xmloff: ODF export: draw:background-size attribute

It will be available in 7.0.0.1.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 39 Commit Notification 2020-05-30 00:03:18 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "libreoffice-7-0":

https://git.libreoffice.org/core/commit/05dfd20fd42f57d681fb65d3fee6b5fe10f4ea1b

tdf#103602 xmloff: ODF export of page style: don't export draw:fill

It will be available in 7.0.0.1.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 40 Regina Henschel 2020-05-30 12:37:05 UTC
Shouldn't LO70 produce valid ODF 1.3? The invalid draw:fill attributes are still in style:page-layout-properties in ODF 1.3 extended. I thought it would be enough to keep the invalid attributes in ODF1.2 extended.
What does the other CCs think?

ODF 1.3 strict, ODF 1.2 strict are OK.
ODF 1.2 extended is as agreed.

draw:background-size is OK.
Comment 41 Michael Stahl (CIB) 2020-06-02 11:30:00 UTC
(In reply to Regina Henschel from comment #40)
> Shouldn't LO70 produce valid ODF 1.3? The invalid draw:fill attributes are
> still in style:page-layout-properties in ODF 1.3 extended. I thought it
> would be enough to keep the invalid attributes in ODF1.2 extended.

the problem with this is that the import commit is only in LO 6.4.5 or later, so a lot of users can't import the drawing-page style yet.

i would wait some time, perhaps until the ODF 1.4 implementation, to remove the export of the invalid attributes in ODF 1.x Extended.
Comment 42 Regina Henschel 2020-06-02 13:01:53 UTC
For older versions the document can be saved in "ODF 1.2 extended". I think, that users of LO 6.3 or older 6.4 versions cannot expect, that those versions can read "ODF 1.3 extended", a new feature of LO 7.0, although that will work in many cases. And we are already after "End of Lifetime" date for LO 6.3. and when LO 7.0 will be released, the version 6.4.5 is already available.
Comment 43 Michael Stahl (CIB) 2020-06-02 14:11:59 UTC
that is all true but we can't compel users to actually upgrade to the latest version.
Comment 44 Thorsten Behrens (CIB) 2020-06-02 15:28:11 UTC
I agree with Michael - I believe erring on the side of pragmatism when it comes to interop is the right approach.
Comment 45 Michael Meeks 2020-06-09 16:16:47 UTC
Agreed with Thorsten (for what it is worth) - though it is not beautiful of course.
Comment 46 Miklos Vajna 2020-06-15 07:40:40 UTC
We have this trade-off that we either conform to the spec fast or we care about existing documents, where users typically expect full backwards-compatibility. I believe we can't meet both requirements at the same time. My understanding is that we already write valid ODF in the non-extended case, and we can phase out the invalid ODF output over time in the extended case as well. So the status quo looks like a reasonable compromise to me.

Next time perhaps we can detect & fix such problems earlier. I think this was introduced in commit 7d9bb549d498d6beed2c4050c402d09643febdfa (Related: #i124638# Second step of DrawingLayer FillAttributes..., 2014-06-02), and now, 6 years later, we can't just fix such things instantly.
Comment 47 Michael Stahl (CIB) 2020-08-18 12:05:41 UTC
there is still an open problem here in ODF 1.2 non-extended, with draw:fill on header/footer, as found by Svante yesterday.

/tmp/70.odt/styles.xml[2,424511]:  Error: unexpected attribute "draw:fill"
fill="none" draw:fill-color="#4f81bd"/></style:footer-style></style:page-layout
Comment 48 Michael Stahl (CIB) 2020-08-20 09:00:36 UTC
oops, i forgot to add the issue id to the commit:

commit dd7caf3cfd3de636c8bf37c8347c1b72b3772aeb (logerrit/master)
Author:     Michael Stahl <Michael.Stahl@cib.de>
AuthorDate: Wed Aug 19 18:12:01 2020 +0200
Commit:     Michael Stahl <michael.stahl@cib.de>
CommitDate: Thu Aug 20 10:56:02 2020 +0200

    xmloff: ODF export of page style: don't export draw:fill
    
    ... attributes on header-style and footer-style in strict ODF.
    
    Change-Id: Ic1af26b6112a5afbb70a82b29dbacd3dcec14ec3
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/101012
    Tested-by: Jenkins
    Reviewed-by: Michael Stahl <michael.stahl@cib.de
Comment 49 Commit Notification 2020-08-20 12:15:15 UTC
Michael Stahl committed a patch related to this issue.
It has been pushed to "libreoffice-7-0":

https://git.libreoffice.org/core/commit/fb87c2ae2d28665f6d95df8ff5ac410db2dccb45

tdf#103602 xmloff: ODF export of page style: don't export draw:fill

It will be available in 7.0.2.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.