Bug 94201 - Dont import blank visibility attribute of <variable> tag
Summary: Dont import blank visibility attribute of <variable> tag
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Documentation (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: Other All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:5.1.0
Keywords:
Depends on:
Blocks: HelpAuthoring-Extension
  Show dependency treegraph
 
Reported: 2015-09-13 21:39 UTC by Yousuf Philips (jay) (retired)
Modified: 2016-10-25 19:21 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
no localize on switch element, visibility on variable only when hidden (3.24 KB, patch)
2015-09-30 21:55 UTC, Regina Henschel
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Yousuf Philips (jay) (retired) 2015-09-13 21:39:19 UTC
When the authoring tool imports <variable> tags it adds a blank visibility attribute to the import which isnt a property used by the tag. The visibility attribute is used by the <ahelp> tag.

Example file - /text/shared/01/02180000.xhp

XML

<variable id="verknuepfungentext">

View in LO

<VAR ID="verknuepfungentext" VISIBILITY="">
Comment 1 Yousuf Philips (jay) (retired) 2015-09-13 23:58:33 UTC
Same thing happens with a localize attribute being added to the <switch> tag. If it is blank, why show a blank attribute.

Example - text/shared/01/02180000.xhp

View in LO

<SWITCH select="sys" localize="">
Comment 2 Regina Henschel 2015-09-28 21:28:18 UTC
In case of 'variable' element the attribute 'visibility' is allowed. To not write 'VISIBILITY=""' a similar '<xsl:choose>' element is needed as it is used for the 'ahelp' element.

The case of 'switch' element is more difficult. In xmlhelp.dtd the attribute 'localize' is not allowed for the 'switch' element. But the comment in the transformation xmlhelp2soffice.xsl mentions it. So first it is needed to examine, whether an attribute 'localize' is currently evaluated for a switch element. If it is evaluated, an '<xsl:choose>' element has to be used and the doctype has to be adapted. If it is not evaluated, this special part in the transformation can be dropped. [ I yet don't know how to examine it.]
Comment 3 Yousuf Philips (jay) (retired) 2015-09-29 09:27:47 UTC
Olivier: Have any input on this?
Comment 4 Regina Henschel 2015-09-29 18:32:23 UTC
I have searched, whether there exists any switch-element with an attribute 'localize' in the current help files. I've found none. Therefore I thing it is save to prevent generating such attribute in the transformation 'xmlhelp2soffice.xsl'.

I have used regex
switch[^>]localize
in \core\helpcontent2\source\text including subfolders using Notepad++
Comment 5 Regina Henschel 2015-09-30 21:55:44 UTC
Created attachment 119150 [details]
no localize on switch element, visibility on variable only when hidden

Do you agree with this solution?
Comment 6 Yousuf Philips (jay) (retired) 2015-10-01 08:03:05 UTC
I'm not knowledgeable enough about xsl, so i'll have to leave it up to others to judge.

@kendy: Can you look over regina's patch?
Comment 7 Jan Holesovsky 2015-10-07 08:40:51 UTC
Regina: Thank you for your patch! :-)  I've pushed the 1st part of that - the "no localize on switch element".

For the 2nd - can we actually do the <xsl:choose> inside the block, like:

<text:variable-set text:name="VAR_" text:value-type="string">
    <xsl:choose>
        <xsl:when test="@visibility='hidden'">
            <xsl:value-of select="concat('&lt;VAR ID=&quot;',@id,'&quot; VISIBILITY=&quot;',@visibility,'&quot;&gt;')"/>
        </xsl:when>
        <xsl:otherwise>
            <xsl:value-of select="concat('&lt;VAR ID=&quot;',@id,'&quot;&gt;')"/>
        </xsl:otherwise>
    </xsl:choose>
</text:variable-set>

so that more code there is shared?
Comment 8 Commit Notification 2015-10-07 08:52:19 UTC
Regina Henschel committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/contrib/dev-tools/commit/?id=cfc05699a5e541f0e0785ed342443506f272255f

tdf#94201: No 'localize' on the 'switch' element.

It will be available in 5.1.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 9 Regina Henschel 2015-10-07 09:49:30 UTC
Jan: That will be possible and I agree, it is a better solution. I'll make you a new patch.
Comment 10 Regina Henschel 2015-10-07 11:57:45 UTC
Proposed patch is in https://gerrit.libreoffice.org/#/c/19223/
Comment 11 Commit Notification 2015-10-07 12:36:37 UTC
Regina Henschel committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/contrib/dev-tools/commit/?id=cfc05699a5e541f0e0785ed342443506f272255f

tdf#94201: No 'localize' on the 'switch' element.
Comment 12 Miklos Vajna 2015-10-07 12:37:38 UTC
Sorry for the second notification, just tested the script if it now omits info about daily builds for dev-tools changes. :-)
Comment 13 Commit Notification 2015-10-07 12:38:04 UTC
Regina Henschel committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/contrib/dev-tools/commit/?id=cfc05699a5e541f0e0785ed342443506f272255f

tdf#94201: No 'localize' on the 'switch' element.
Comment 14 Miklos Vajna 2015-10-07 12:39:40 UTC
(Sorry again for the second one, I promise not to spam here anymore. ;-) )
Comment 15 Commit Notification 2015-10-07 12:48:34 UTC
Regina Henschel committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/contrib/dev-tools/commit/?id=7876235adbb4b3da4a0e7e809524b21d4fb47c88

tdf#94201 Dont import blank visibility attribute of <variable> tag
Comment 16 Jan Holesovsky 2015-10-07 12:49:43 UTC
Both is now in, I think we can close this bug.  Thank you, Regina!