As Eike indicated in https://bugs.documentfoundation.org/show_bug.cgi?id=146404#c4
COLOR format codes are preserved in ods/fods format.
The pb was the missing map back.
Indeed, we had the mapping for [COLOR<something>] => Color value (eg: "ffaa16")
but not the reverse mapping Color value => [COLOR<something>]
Eike had provided the right code pointer in xmlnumfi.cxx.
First, I thought using PaletteManager but this one is in svx and svx shouldn't be used in xmloff
(dependency pb).
Then I used ChatGpt (GPT-5.6 Luna) and it pointed the right process.
It just consisted to use a similar callback system (DECL_LINK, IMPL_LINK, LINK) of GetUserDefinedColor.
In addition, by testing all this, I noticed that maList in XColorList::GetIndexOfColor was empty.
So I added the Load call in case mbListDirty is true.
Change-Id: I0ef6b5901f2e4c757e88994412dbf79a13100ffd
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/210518
Reviewed-by: Julien Nabet <serval2412@yahoo.fr>
Tested-by: Jenkins
This fixes a 24.2 regression from Khalid's commit 4b743de97f.
The problem I'm trying to fix is that setting optimal column width
is not necessarily providing enough width to avoid seeing '###'.
The reason is that GetOptimalColumnWidth calls GetTextWidth
with MapMode disabled, so no subpixel calculations were done.
However LayoutString does have MapMode enabled,
so it calculates a different length than what optimal column got.
Surely that must always be wrong. But Khalid's similarly more-correct
commit triggered a lot of 'regressions'. However, this should be
moving in the right direction as tdf#168002 suggests
that mbSubpixelPosition should always be true in the future.
Since it is needed for width, it seems reasonable that it should
also be done for height...
Change-Id: I30590ebba8b3027c5214ee075a1a258d3a61b8c7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/210438
Tested-by: Jenkins
Reviewed-by: Justin Luth <jluth@mail.com>
* Recognize LAMBDA during dynamic array analysis; LAMBDA
never produces an array, but it can be part of a formula
that generates one.
* Fix handling of LET; subexpressions before the last should
not affect the result, unless we are willing to to a more
thorough analysis.
* Update test to check for the new behavior.
* Account for svExternalDoubleRef.
Change-Id: Ia9a4761079de986906ac70252066d8d9b4655f59
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/210461
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
Before, it generally wasn't saving ANYTHING
because rMark is normally not marked,
and even if it is, it could be random.
The only time rMark means anything is if rSearchItem.GetSelection().
But even that doesn't help because the undo has no idea of range.
ScUndoReplace::Undo attempts to undo the entire document,
so definitely more than bOnlyMarked needs to be included.
In fact, even sheets that don't even have a match
need to be backed up. Entirely.
This seriously needs to be looked at by a Calc Pro
who could optimize this by passing rMatchedRanges
to ScUndoReplace. And only CopyToDocument a RangeList.
But of course, there is also bMatchedRangesWereClamped...
which thankfully doesn't affect styles, only text.
Change-Id: I2087e255e3e4b3387b388e1f2d28c19717e50342
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/210247
Tested-by: Jenkins
Reviewed-by: Justin Luth <jluth@mail.com>
We have added various workarounds for ScPatternAttr use after free
issues since:
commit 2e1f9da8a6
Date: Sat Dec 23 15:52:06 2023 +0100
Decouple ScPatternAttr from SfxItemPool
separated ScPatternAttr from SfxItemPool.
Prior to that an ScPatternAttr was an SfxPoolItem held in the doc's
SfxItemPool which kept every item alive for the document's whole
lifetime, so any raw const ScPatternAttr* was basically safe.
The various workarounds for the new behaviour were:
commit dca19993a4c732e95b9a47e1abdcaad6efbeebe9
Date: Sat Jan 6 14:40:28 2024 +0100
tdf#159042: crash after changing cell style (if direct formatting done)
commit 19437715eb415fe2c40b8b40dbe04ddca28f9a35
Date: Thu Sep 12 12:06:28 2024 +0100
null pattern deref seen in ScViewData::SetEditEngine
commit e384f2e8de9603ee65a3edc6dcb50b07116ee4fc
Date: Mon Mar 10 14:45:22 2025 +0000
crashtesting: crash on reimport of xlsx output of forum-pl-2758.ods
commit 7999585bd8378f1a8d04ea2a9a93a8e0bf4d8eb6
Date: Fri May 9 08:50:17 2025 +0100
crash on null pattern de-ref
commit 1bf9bbea96
Date: Tue Oct 28 16:53:49 2025 +0300
tdf#169115: sc: fix use-after-free bug in ScDocument::FillInfo
commit 9f366af81478ad218dc2ef2e37423a9ebcc84e73
Date: Sat Apr 25 14:47:02 2026 +0200
ScPatternAttr use-after-free
commit 20fe691a18fb35120d0765dd1519acd6d2986c38
Date: Tue Apr 28 08:33:34 2026 +0200
ScPatternAttr use-after-free (2)
This commit picks the class member fields that are still raw
ScPatternAttr* and converts them to CellAttributeHolder:
ScUsedAreaIterator::pPattern and pFoundPattern
ScInputHandler::pLastPattern
LotAttrCol::ENTRY::pPattAttr
ScPatternCache::Entry::pPattern
The CalcAll() workaround from 9f366af81478 then doesn't seem to be needed(?)
A new compiler plugin catches class member fields of raw type
ScPatternAttr pointer.
Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: Ibb9f28b8fcaa5470c7a82eee52900c2d60434e41
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/3163
Tested-by: Jenkins CPCI <releng@collaboraoffice.com>
Reviewed-by: Noel Grandin <noel.grandin@collabora.com>
(cherry picked from commit 9c7b6313a29067bf0eef12884fc3ce7efb0374e5)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/209822
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
Tested-by: Jenkins
Allocate and fill in the mpaMemberSlimArray.
In the eager init case, where everything is initially allocated
as Slim results, it uses less memory just to create a simple
linear array rather than storing a pointer for each one.
Fixup the promotion code to move between the arrays rather
than change the pointer in the MemberArray.
On tdf#126710 this saves ~11.5GB (maxresident)
tdf#100244 (full case) this saves about 6.3GB (max resident)
In tdf#126710's case, the 'virt' column in top appeared to save
even more; possibly due to large reallocations in the vector that's
been removed?
Change-Id: I7681304310e060da191edc1fb243900ce59193a7
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/209101
Tested-by: Jenkins
Reviewed-by: David Gilbert <freedesktop@treblig.org>
Convert lcl_SearchMember to use a templated core since
we're soon going to have two different types to iterate over
(One of which is a pointer, the other an object, hence
needing to parameterise the get function as well).
Change-Id: I4df6a011015ca917f5ad015393ff0f65b2a32ca9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/209099
Reviewed-by: David Gilbert <freedesktop@treblig.org>
Tested-by: Jenkins
Define mpaMemberSlimArray as a span of ScDPResultMemberSlim's,
we'll use this in future instead of the Vector that might
hold pointers to any subclass of ScDPResultMember.
Make GetMember and GetMemberCount reference it.
The actual allocation of the array comes later.
Change-Id: Id2abf23e72a92fee0dc3e4326370692a3567dafd
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/209098
Tested-by: Jenkins
Reviewed-by: David Gilbert <freedesktop@treblig.org>
Use GetMember() for most cases rather than directly
accessing the member array, since soon we'll have
two member arrays.
Change-Id: I76eca012af6743b29de1e53e96a2ff46187f8a8a
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/209097
Tested-by: Jenkins
Reviewed-by: David Gilbert <freedesktop@treblig.org>
Use GetMemberCount() rather than accessing maMemberArray.size()
directly in most cases, since we're going to have two different
arrays.
Change-Id: I35541b84e3e1704c9725491444b076c05197aa27
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/209096
Tested-by: Jenkins
Reviewed-by: David Gilbert <freedesktop@treblig.org>
The hand coded linear array search in FindMember can be replaced with
std::ranges::find_if.
Note: A few months ago I added a comment saying I thought the 'IsNamedItem' check
above wass probably always true - it turns out it's not - which causes the linear
array to be used!
(I can't say I understand the case IsNamedItem is false, here, it
triggers in the tdf#126710 test)
Change-Id: Ia87998f910d1bbf9f120d70acf096d17d4ce4211
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/209095
Reviewed-by: David Gilbert <freedesktop@treblig.org>
Tested-by: Jenkins
The block frames of the pivot table output used a 1pt line inside
the table and a 2pt line on the outer edges of the table, and the
field cell frames used the 1pt line. Draw all of them with the
thin line width (0.75pt) instead, which is close to the line width
that the default OOXML pivot table style uses, so the whole table
uses one slim border.
Signed-off-by: Tomaž Vajngerl <tomaz.vajngerl@collabora.co.uk>
Change-Id: Iec99b6890ffa5c8dfc699a30e2fc58d452e5d9ff
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/8255
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Tested-by: Tomaž Vajngerl <tomaz.vajngerl@collabora.com>
(cherry picked from commit 411cfdb91ee02a10361c3d72e13bd64161bba4d5)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/209432
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
The pivot table output set pivot table specific cell styles on its
cells ("Pivot Table Value", "Pivot Table Result", ...) and created
them in the document on first use. Remove this, so adding a pivot
table no longer adds any cell styles.
The attributes that these styles carried are applied as direct
formatting instead. The member cells are aligned to the left, and
the subtotal and grand total labels and results are bold. The left
alignment is also what makes the member indents visible, an indent
has no effect in a centered cell. The style name mapping in
stylehelper.cxx stays, so documents that already contain these
styles keep working.
Signed-off-by: Tomaž Vajngerl <tomaz.vajngerl@collabora.co.uk>
Change-Id: Ib33d170d2da38a60c0739eda8fa79ce4f1fcf745
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/8254
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Tested-by: Tomaž Vajngerl <tomaz.vajngerl@collabora.com>
(cherry picked from commit fee8a092ac3388c873d2b0763f7ce3d8cd032b76)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/209431
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
Tested-by: Jenkins
The formatting of the pivot table output was spread across
ScDPOutput: cell styles and frames were applied inline while the
table content was written, and ScDPOutputImpl drew the block
frames.
Add sc::pivot::StyleOutput, which handles all the drawing. ScDPOutput
now only records the sections of the table (layout geometry, field
cells, member spans, subtotal and grand total rows and columns,
indents and expand or collapse buttons). The cell styles are applied
while the sections are recorded, at the same points where they were
applied before, so they always come before any direct formatting
(number formats, pivot table format attributes) reaches the cells.
The block frames, the indents and the expander buttons are then drawn
by apply() in one pass over the collected sections. The drawn output
stays the same as before, and a different look, like the OOXML pivot
table styles, can later be drawn from the same sections.
Signed-off-by: Tomaž Vajngerl <tomaz.vajngerl@collabora.co.uk>
Change-Id: Ia628de6badf511d299f36bf474194ecad17a9c56
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/8253
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>
Tested-by: Tomaž Vajngerl <tomaz.vajngerl@collabora.com>
(cherry picked from commit 9f5a36c822b170835c55143716be9eca28ac3e91)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/209430
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
pResultData is constant in ScDPResultDimension, so we don't need
to store it in ScDPResultMemberSlim, just ask the dimension.
Signed-off-by: Dr. David Alan Gilbert <dave@treblig.org>
Change-Id: I0b9f9081c9ffcc0c285e546a4cfb2784315d0c3d
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/208635
Tested-by: Jenkins
Reviewed-by: David Gilbert <freedesktop@treblig.org>
A matrix formula cell holds the "top-left" result of its matrix in the
reference-counted xUpperLeft of ScMatrixFormulaCellToken.
CloneUpperLeftIfNecessary cloned that only for svDouble; other types
were kept by shared reference.
But threaded group calculation switches every token in the group's shared pCode
to RefCntPolicy::None while worker threads interpret it, on the basis that
interpreting only reads pCode, since:
commit 791a2489ee
Date: Sun Mar 24 21:31:38 2024 +0000
Related: tdf#160056 elide ref counting for initial tokens
A matrix formula whose result is a string constant from its own pCode
broke that: the pCode token was taken as the upper left during the None
window, so its IncRef was a no-op.
At teardown the matrix released it and freed a token the pCode array
still pointed at.
Where the uncounted reference is taken:
#1 boost::intrusive_ptr<formula::FormulaToken const>::operator=
boost/smart_ptr/intrusive_ptr.hpp:121
#2 ScMatrixFormulaCellToken::Assign sc/source/core/tool/token.cxx
#3 ScFormulaResult::SetToken sc/source/core/tool/formularesult.cxx:206
#4 ScFormulaResult::Assign sc/source/core/tool/formularesult.cxx:171
#5 ScFormulaCell::InterpretTail sc/source/core/data/formulacell.cxx:2476
#6 ScColumn::CalculateInThread sc/source/core/data/column2.cxx:3266
#7 ScTable::CalculateInColumnInThread sc/source/core/data/table1.cxx:2715
#8 ScDocument::CalculateInColumnInThread sc/source/core/data/documen8.cxx:420
#9 ...Executor::doWork() sc/source/core/data/formulacell.cxx:5437
#10 comphelper::ThreadTask::exec
Clone the upper left when it is svDouble (as before) or not reference counted
(RefCntPolicy::None). The clone is a counted token the cell owns, so no
reference into pCode is taken during the threaded window. The non-threaded
path is unchanged.
Also assert in HandleStuffAfterParallelCalculation that a matrix result's
upper left does not alias the cell's own pCode.
Found with an ASan+UBSan build reloading crashtest document
forum-mso-en4-676909.
The original motivating tdf#160056 benchmark document never visits this
clone branch, so that case remains unchanged.
Signed-off-by: Caolán McNamara <caolan.mcnamara@collabora.com>
Change-Id: Ibd9e93aa9e59fabb99941a92a16aef247f7ae160
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/5638
Tested-by: Jenkins CPCI <releng@collaboraoffice.com>
Reviewed-by: Noel Grandin <noel.grandin@collabora.com>
(cherry picked from commit 2a70b7a75ce8768e077ea42175e59465d3adba31)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/208769
Tested-by: Jenkins
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
Otherwise, the current tab will be cleared and the inserted cell value
will be silently dropped.
Change-Id: I0e27d1944d148afdf06cdac469db74140b5274c0
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/208535
Tested-by: Jenkins
Reviewed-by: Andreas Heinisch <andreas.heinisch@yahoo.de>
A Ctrl+V changes a cell's content, not whether it is locked. Copying a
protected cell onto a cell the user had directly unprotected used to carry
the Protected flag along, so the target could no longer be edited, the
opposite of what Excel does and what tdf#123974 asks for.
Throughout this change "the cell's protection" means the protection set as
direct formatting on the cell itself. Protection can also come from a cell
style, and conditional formatting sets protection precisely by applying
such a style; those two are one and the same mechanism and, as noted at
the end, are deliberately left untouched here.
The two reports in this area pull in opposite directions: tdf#123974 wants
the direct flag dropped on paste, tdf#150910 wants it always retained, and
they were noted as contradictory. A rule gated on sheet protection (drop
the direct flag only when the sheet is protected and the target was
unprotected) was considered and rejected, because its outcome depends on
the order of otherwise unrelated steps: directly unprotecting a cell,
pasting into it, then protecting the sheet would re-lock it, while
protecting the sheet first and pasting after would not. That is impossible
to explain to a user.
Instead treat a cell's direct protection as a property of the cell that an
interactive paste never changes. Snapshot the destination's directly set
cell protection before the paste and restore it afterwards, so the pasted
attributes overwrite everything except that lock state. Routing both the
replicated-single-row fast path and the general path through the same
restore also removes the single-row versus multi-row inconsistency of
tdf#150910, which shared the same root cause.
Only directly applied protection is preserved. Protection that instead
comes from a cell style, including a style applied dynamically by
conditional formatting, is not touched and keeps following the usual paste
(in theory this could be extended to the effective protection if a need
shows up). The programmatic copyRange path is left unchanged and still
copies attributes verbatim; only an interactive paste opts in. A moved
cell (cut and paste) keeps its own protection and is excluded.
Signed-off-by: Mike Kaganski <mike.kaganski@collabora.com>
Change-Id: I3a1ec141c3ec698858ca8d058e6d9b1e9126402f
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/7069
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins CPCI <releng@collaboraoffice.com>
(cherry picked from commit 39f30bd4b532f7575957210b644216afa14ac28b)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/208344
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
Tested-by: Jenkins
ARRAYTOTEXT returns a cell range or array as one text string,
taking the range and an optional format number.
Format 0 (or omitted) joins the values in row order with ", ".
Format 1 gives an array constant: braces around the array, ","
between columns, ";" between rows, and text quoted with any
inner double quote doubled. Any other format returns #VALUE!.
A number keeps its input-line text, a boolean reads TRUE or
FALSE in the current locale, an empty cell adds nothing, and an
error in a range lists as its error text while a reference to a
single error cell returns that error.
The function is wired up with its name tables, wizard entry,
parameter classification, and OOXML and binary filter mappings.
A self-checking OOXML workbook is verified after import and
after an XLSX and an ODS round trip, each with a recalc, and the
exported files are checked for the ARRAYTOTEXT formula. A ucalc
test covers the locale-dependent boolean text for a value and
a range.
Signed-off-by: Tomaž Vajngerl <tomaz.vajngerl@collabora.co.uk>
Change-Id: I0cdd9737a51f402e39cf9a1346f4c5326680fe8f
Reviewed-on: https://gerrit.collaboraoffice.com/c/online/+/7219
Reviewed-by: Miklos Vajna <vmiklos@collabora.com>
Tested-by: Jenkins CPCI <releng@collaboraoffice.com>
(cherry picked from commit f66673d65ee307f17512a4239dd4b52ac189ac20)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/208395
Reviewed-by: Xisco Fauli <xiscofauli@libreoffice.org>
Tested-by: Jenkins
Andrea Gelmini