Bug 97102 - Re-work copy/paste 'orrible extensions dialog code...
Summary: Re-work copy/paste 'orrible extensions dialog code...
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Extensions (show other bugs)
Version:
(earliest affected)
5.1.0.1 rc
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: reviewed:2023
Keywords: difficultyInteresting, easyHack, skillCpp, topicCleanup
Depends on:
Blocks: Dialog
  Show dependency tree / graph
 
Reported: 2016-01-13 14:44 UTC by Michael Meeks
Modified: 2023-04-20 13:48 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Meeks 2016-01-13 14:44:00 UTC 
There is a chunk of code in:

desktop/source/deployment/gui/dp_gui_dialog2.cxx

with some pretty bad practice on display. There is a large chunk of cut-and-paste coding between:

IMPL_LINK_NOARG_TYPED(UpdateRequiredDialog, TimeOutHdl, Idle *, void)
and
IMPL_LINK_NOARG_TYPED(ExtMgrDialog, TimeOutHdl, Idle *, void)

And that code itself should -not- be in an idle handler that runs all the time. Instead we should add an idle task whenever anything changes.

The reason the code is done like this is to avoid needing to take the SolarMutex in the implementation that updates the progress etc. which is useful in several cases for UNO components etc.

Anyhow - we should create a new (inside VCL) "ProgressBarAsync" class and encapsulate this functionality into VCL.

The methods - such as updating the progress %age, and the text etc. should take effect only asynchronously in an idle handler -but- the idle handler should only be triggered when some state actually changes =) a new %age, new text etc.

Then we should clean the cut/paste coding out of the above module.

Thanks !
Comment 1 Robinson Tryon (qubit) 2016-02-18 14:52:28 UTC Comment hidden (obsolete)
Comment 2 Xisco Faulí 2017-07-26 19:19:58 UTC 
Hi Michael,
it seems the code pointers are no longer valid.
Could you please update them?
Comment 3 Michael Meeks 2019-07-19 16:26:25 UTC 
Looks like it's still there to me:


//UpdateRequiredDialog
UpdateRequiredDialog::UpdateRequiredDialog(weld::Window *pParent, TheExtensionManager *pManager)
    : GenericDialogController(pParent, "desktop/ui/updaterequireddialog.ui", "UpdateRequiredDialog")
    , DialogHelper(pManager->getContext(), m_xDialog.get())
    , m_sCloseText(DpResId(RID_STR_CLOSE_BTN))
    , m_bHasProgress(false)
    , m_bProgressChanged(false)
    , m_bStartProgress(false)
    , m_bStopProgress(false)
    , m_bHasLockedEntries(false)
    , m_nProgress(0)

and

ExtMgrDialog::ExtMgrDialog(weld::Window *pParent, TheExtensionManager *pManager)
    : GenericDialogController(pParent, "desktop/ui/extensionmanager.ui", "ExtensionManagerDialog")
    , DialogHelper(pManager->getContext(), m_xDialog.get())
    , m_sAddPackages(DpResId(RID_STR_ADD_PACKAGES))
    , m_bHasProgress(false)
    , m_bProgressChanged(false)
    , m_bStartProgress(false)
    , m_bStopProgress(false)

Look too similar. Should be some shared base-class or another helper class for some of what they want to do with progress.

The idle handlers are still there but didn't read them yet.
Comment 4 Hossein 2023-04-20 13:48:03 UTC 
Re-evaluating the EasyHack in 2023

This enhancement is still relevant. Both of the UpdateRequiredDialog::UpdateRequiredDialog() and ExtMgrDialog::ExtMgrDialog() are still in the above mentioned file without much change. We don't have ProgressBarAsync class yet.