Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v82pre: Sniffer BibTeX control woes #82

Closed
GerHobbelt opened this issue Oct 5, 2019 · 2 comments
Closed

v82pre: Sniffer BibTeX control woes #82

GerHobbelt opened this issue Oct 5, 2019 · 2 comments
Labels
🐛bug Something isn't working 🕵TLC Needs some special attention
Milestone

Comments

@GerHobbelt
Copy link
Collaborator

Follow up on

  • (cdd51d6) Sniffer: BibTeX editor pane can toggle between formatted/parsed field editor mode and raw BibTeX editor mode.

    Known issue: the little green cross that is the UI element to toggle editor modes
    has the nauseating behaviour of moving along up & down and thus hiding behind
    the sniffer ok/fail/skip/clean buttons at top-right.

The UI has been prepped for this with two extra buttons (the second for another purpose and github issue) but the functionality to relinquish the toggle control to these buttons instead hasn't been built in yet. (See (1) in screenshot below)

2019-10-05-(2)-redux

@GerHobbelt GerHobbelt added the 🐛bug Something isn't working label Oct 5, 2019
@GerHobbelt GerHobbelt added this to the v82 milestone Oct 5, 2019
@GerHobbelt GerHobbelt added the 🕵TLC Needs some special attention label Oct 5, 2019
GerHobbelt added a commit to GerHobbelt/qiqqa-open-source that referenced this issue Oct 19, 2019
GerHobbelt added a commit to GerHobbelt/qiqqa-open-source that referenced this issue Oct 19, 2019
GerHobbelt added a commit to GerHobbelt/qiqqa-open-source that referenced this issue Oct 19, 2019
…oring/rework of the BibTeX related UI bits. Includes some minimal prepwork for jimmejardine#87.
GerHobbelt added a commit to GerHobbelt/qiqqa-open-source that referenced this issue Oct 22, 2019
…HA-1: 0f9fa67 :: adding icons as part of jimmejardine#82 refactoring/rework of the BibTeX related UI bits.
GerHobbelt added a commit to GerHobbelt/qiqqa-open-source that referenced this issue Oct 22, 2019
…s users: those must now provide the toggle/view-errors/etc. buttons for the control, so that the parent has full control over the layout, while the BibTeXEditorControl itself will set the icons, tooltip, etc. for each of those buttons to ensure a consistent look of the BibTeX editor buttons throughout the application.

TODO: see if we need to discard those registered buttons in the Unload event to ensure we're not memleaking...
GerHobbelt added a commit to GerHobbelt/qiqqa-open-source that referenced this issue Oct 22, 2019
…s need to scale down their icons and **text** when they're part of a resizable panel in order to remain readable, we need to implement that and ensure it only happens where and when we want. TODO: fix more panels with AugmentedButton nodes as they are now already scaled down due to WPF autosize actions which are not meant to do that.
GerHobbelt added a commit to GerHobbelt/qiqqa-open-source that referenced this issue Oct 22, 2019
… DO NOT need scaling (at least not yet in our perception of the UI) are AutoTextScale=false(default) configured. The ones which need to scale to remain legible at various screen and panel sizes are marked AutoTextScale=true.
@GerHobbelt
Copy link
Collaborator Author

After the commits above, there's another issue now: when the bibTeX raw content is HUGE, e.g. when having imported it from PubMed, where the entire PubMed XML is stored as comment below the BibTeX record itself, then you get a very high BibTeX editor panel, which is pretty user-unfriendly.

Hence we should maximize the editor height, just in case...

GerHobbelt added a commit to GerHobbelt/qiqqa-open-source that referenced this issue Oct 22, 2019
…ious controls/wrappers so that huge BibTeX raw content doesn't produce unworkable UI layouts and thus bad UX. MinWidth limits are meant to restrict button scaling and thus ugly/damaged looking UI, while MaxHeight limits are intended to limit the height of the BibTeXEditorControl when it is fed huge BibTeX raw content, such as when loading a BibTeX from converted PubMed XML (the source XML is appended as a multiline comment which often is very large)
GerHobbelt added a commit to GerHobbelt/qiqqa-open-source that referenced this issue Oct 31, 2019
…ormance improvements work:

- fixed jimmejardine#121 : this happened due to a slightly over-eager `Dispose()` I introduced previously (:-() which reset the BibTeX content to an empty string, followed up immediately by a still-registered change event handler, which would communicate this 'change' to anyone listening, thus nuking the BibTeX metadata for the given Document.
- fixed jimmejardine#82 : part of that work has been done in earlier commits, where the size of of the editor panel (**height**) is designed to stick with the size of the 'fields editor mode' subpanel height so as not to jump up & down while you toggle edit modes. Also done in earlier commits: the RAW editor *wraps* the BibTeX text data so there's no need for a *horizontal* scroller any more; this should make diting in RAW mode a little more palatable, at least it is in my own experience.
- Just like commit SHA-1: a540e50, there's more IDisposable work done following the new DevStudio Code Analysis reports while I was hunting memory leaks and hunting down causes of jimmejardine#112
- Tweaked the initial library scan to first search all Qiqqa 'known_web_libraries' config files it can find in the Qiqqa libraries' **base directory**: this should help folks (like me) who wish to recover their old/crashing Qiqqa libraries, which previously would cause Qiqqa v79 and earlier Commercial Versions to crash in all sorts of spectacular ways - mostly due to buggy PDF documents sneaking into the libraries via Sniffer download b0rks and other sources of rottenness (such as the websites themselves).
  The positive effect of this change should be a stable list of libraries with as many of the original Web Libraries' Names restored as possible.
- All libraries are flagged Read/Write instead of marking 'Web Libraries' as ReadOnly, which would block any editing/updating of those libs. As Open Source Qiqqa does not support Commercial Web Libraries in the way they were meant before (as syncable Qiqqa-based Cloud storage), you're working on 'independent copies' anyway, while we have to come up with other means to sync libraries like that. As these buggers can grow huge (mine is 20GB+), free cloud solutions (OneDrive, DropBox, etc.) with their 5GB limits are not a truely viable option. Alas, something to think about. **TODO**
- Done another Code Review, scanning for all sorts of spots where the C#/.NET code needs a `using(...){...}` statement or something similar to ensure the allocated memory is actually *released when done*; this conjoins with the IDisposable work done in this commit.
- LibrarySyncManager: we now cache the PDF Document Size as we already did with the PDF Document 'File Exists' flag. This should at least reduce the running cost of subsequent invocations of the Sync Details dialog after its first run, when that data is collected.
- Performance: for large libraries, the initial load time was extreme, particularly when Qiqqa has 'remembered' that the library was open in its own Library View panel/tab. This is due to two major load factors: the BibTeX record for every document is parsed as part of deriving an AugmentedTitle and AugmentedAuthor set for display. Meanwhile, the library will be initially sorted by *date*, which took an *inordinate* amount of time as every date comparison would access and *(re)parse* the raw text date fields as obtained from the database. Now these parsed dates are cached in the PDFDocument until the cached value(s) are reset by the dates being modified within Qiqqa.
- Performance: for large libraries, the Sync Details dialog would take an extreme amount of time, with the UI *locked*. Now we set the busy bee / wait cursor to indicate work is being done, while the work has been offloaded onto a background task. Also, while the PDF Document 'File Exists' state was cached in the PDFDocument record, the *size* of the document was not and thus was (re)calculated every time the user would invoke this dialog, resulting in huge delays as thousands of files' filesize info was (re)fetched from the disk on every invocation of the dialog. This has now been alleviated at least for *subsequent invocations* as the File Size is now also cached next to the File Exists datum in PDFDocument.

- Here's the set of Code Analysis reports that were tackled in this commit:

warning CA1001: Type 'BibTeXEditorControl' owns disposable field(s) 'wdpcn' but is not disposable
warning CA1001: Type 'CSLProcessorOutputConsumer' owns disposable field(s) 'web_browser' but is not disposable
warning CA1001: Type 'FolderWatcher' owns disposable field(s) 'file_system_watcher' but is not disposable
warning CA1001: Type 'GoogleBibTexSnifferControl' owns disposable field(s) 'ObjWebBrowser, pdf_renderer_control' but is not disposable
warning CA1001: Type 'Library' owns disposable field(s) 'library_index' but is not disposable
warning CA1001: Type 'LibraryCatalogOverviewControl' owns disposable field(s) 'library_index_hover_popup' but is not disposable
warning CA1001: Type 'MainWindow' owns disposable field(s) 'ObjStartPage' but is not disposable
warning CA1001: Type 'PDFAnnotationNodeContentControl' owns disposable field(s) 'library_index_hover_popup' but is not disposable
warning CA1001: Type 'PDFDocumentNodeContentControl' owns disposable field(s) 'library_index_hover_popup' but is not disposable
warning CA1001: Type 'PDFPrinterDocumentPaginator' owns disposable field(s) 'last_document_page' but is not disposable
warning CA1001: Type 'ReadOutLoudManager' owns disposable field(s) 'speech_synthesizer' but is not disposable
warning CA1001: Type 'TagEditorControl' owns disposable field(s) 'wdpcn' but is not disposable
warning CA1044: Because property AutoArrange is write-only, either add a property getter with an accessibility that is greater than or equal to its setter or convert this property into a method.
warning CA1044: Because property ConciseView is write-only, either add a property getter with an accessibility that is greater than or equal to its setter or convert this property into a method.
warning CA1044: Because property DatesVisible is write-only, either add a property getter with an accessibility that is greater than or equal to its setter or convert this property into a method.
warning CA1044: Because property DefaultWebSearcherKey is write-only, either add a property getter with an accessibility that is greater than or equal to its setter or convert this property into a method.
warning CA1044: Because property Entries is write-only, either add a property getter with an accessibility that is greater than or equal to its setter or convert this property into a method.
warning CA1044: Because property ImagePath is write-only, either add a property getter with an accessibility that is greater than or equal to its setter or convert this property into a method.
warning CA1044: Because property Items is write-only, either add a property getter with an accessibility that is greater than or equal to its setter or convert this property into a method.
warning CA1044: Because property Library is write-only, either add a property getter with an accessibility that is greater than or equal to its setter or convert this property into a method.
warning CA1044: Because property OnAddedOrSkipped is write-only, either add a property getter with an accessibility that is greater than or equal to its setter or convert this property into a method.
warning CA1044: Because property PageNumber is write-only, either add a property getter with an accessibility that is greater than or equal to its setter or convert this property into a method.
warning CA1044: Because property PaperSet is write-only, either add a property getter with an accessibility that is greater than or equal to its setter or convert this property into a method.
warning CA1044: Because property PDFAnnotation is write-only, either add a property getter with an accessibility that is greater than or equal to its setter or convert this property into a method.
warning CA1044: Because property PDFDocument is write-only, either add a property getter with an accessibility that is greater than or equal to its setter or convert this property into a method.
warning CA1044: Because property TagsTitleVisibility is write-only, either add a property getter with an accessibility that is greater than or equal to its setter or convert this property into a method.
warning CA1052: Type 'AlternativeToReminderNotification' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'BookmarkManager' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'Choices' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'CitationFinder' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'CSLProcessor' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'EndnoteImporter' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'ExpeditionBuilder' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'ExportingTools' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'Features' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'GeckoInstaller' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'GeckoManager' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'IdentifierImplementations' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'ImportingIntoLibrary' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'Interop' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'LibraryExporter' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'LibraryPivotReportBuilder' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'LibrarySearcher' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'LibraryStats' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'ListFormattingTools' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'MainEntry' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'MendeleyImporter' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'MYDBlockReader' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'PDFCoherentTextExtractor' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'PDFDocumentTagCloudBuilder' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'PDFMetadataExtractor' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'PDFMetadataInferenceFromOCR' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'PDFMetadataInferenceFromPDFMetadata' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'PDFMetadataSerializer' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'PDFPrinter' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'PDFSearcher' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'PDFTools' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'QiqqaManualTools' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'RecentlyReadDocumentManager' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'SampleMaterial' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'ScreenSize' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'SimilarAuthors' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'StandardHighlightColours' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'SyncConstants' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'TempDirectoryCreator' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'UpgradeManager' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'VanillaReferenceCreating' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'WebLibraryDocumentLocator' is a static holder type but is neither static nor NotInheritable
warning CA1052: Type 'WebsiteAccess' is a static holder type but is neither static nor NotInheritable
warning CA1063: Ensure that 'BrainstormControl.Dispose' is declared as protected, virtual, and unsealed.
warning CA1063: Ensure that 'ChatControl.Dispose' is declared as protected, virtual, and unsealed.
warning CA1063: Ensure that 'CSLProcessorOutputConsumer.Dispose' is declared as protected, virtual, and unsealed.
warning CA1063: Ensure that 'FolderWatcher.Dispose' is declared as protected, virtual, and unsealed.
warning CA1063: Ensure that 'LibraryIndex.Dispose' is declared as protected, virtual, and unsealed.
warning CA1063: Ensure that 'LibraryIndexHoverPopup.Dispose' is declared as protected, virtual, and unsealed.
warning CA1063: Ensure that 'MainWindow.Dispose' is declared as protected, virtual, and unsealed.
warning CA1063: Ensure that 'PDFReadingControl.Dispose' is declared as protected, virtual, and unsealed.
warning CA1063: Ensure that 'PDFRendererControl.Dispose' is declared as protected, virtual, and unsealed.
warning CA1063: Ensure that 'ReadOutLoudManager.Dispose' is declared as protected, virtual, and unsealed.
warning CA1063: Ensure that 'ReportViewerControl.Dispose' is declared as protected, virtual, and unsealed.
warning CA1063: Ensure that 'SceneRenderingControl.Dispose' is declared as protected, virtual, and unsealed.
warning CA1063: Ensure that 'SpeedReadControl.Dispose' is declared as protected, virtual, and unsealed.
warning CA1063: Ensure that 'StartPageControl.Dispose' is declared as protected, virtual, and unsealed.
warning CA1063: Ensure that 'TagEditorControl.Dispose' is declared as protected, virtual, and unsealed.
warning CA1063: Ensure that 'WebBrowserControl.Dispose' is declared as protected, virtual, and unsealed.
warning CA1063: Ensure that 'WebBrowserHostControl.Dispose' is declared as protected, virtual, and unsealed.
warning CA1721: The property name 'Annotations' is confusing given the existence of method 'GetAnnotations'. Rename or remove one of these members.
warning CA1802: Field 'XXXXXX' is declared as 'readonly' but is initialized with a constant value. Mark this field as 'const' instead.
warning CA1812: XXXXXX is an internal class that is apparently never instantiated. If so, remove the code from the assembly. If this class is intended to contain only static members, make it static (Shared in Visual Basic).
warning CA1827: Count() is used where Any() could be used instead to improve performance.
warning CA2000: Call System.IDisposable.Dispose on object created by 'Instance.OpenNewBrainstorm()' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'MainWindowServiceDispatcher.Instance.OpenDocument(cloned_pdf_document)' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'MainWindowServiceDispatcher.Instance.OpenDocument(ddw.pdf_document)' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'MainWindowServiceDispatcher.Instance.OpenDocument(matching_bibtex_record.pdf_document)' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'MainWindowServiceDispatcher.Instance.OpenDocument(out_pdf_document, out_pdf_annotation.Page)' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'MainWindowServiceDispatcher.Instance.OpenDocument(pdf_document)' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'MainWindowServiceDispatcher.Instance.OpenDocument(pdf_document, annotation_work.pdf_annotation.Page)' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'MainWindowServiceDispatcher.Instance.OpenDocument(pdf_document, true)' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'MainWindowServiceDispatcher.Instance.OpenDocument(pdf_document_node_content.PDFDocument)' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'MainWindowServiceDispatcher.Instance.OpenDocument(PDFDocumentBindable.Underlying, LibraryCatalogControl.FilterTerms)' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'MainWindowServiceDispatcher.Instance.OpenDocument(PDFDocumentBindable.Underlying, search_result.page, LibraryCatalogControl.FilterTerms, false)' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'MainWindowServiceDispatcher.Instance.OpenDocument(selected_pdf_document)' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'MainWindowServiceDispatcher.Instance.OpenDocument(tag.pdf_document)' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'MainWindowServiceDispatcher.Instance.OpenNewBrainstorm()' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'MainWindowServiceDispatcher.Instance.OpenSampleBrainstorm()' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'MainWindowServiceDispatcher.Instance.OpenWebBrowser()' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'new Bitmap(ms)' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'new CSLProcessorOutputConsumer(BASE_PATH, citations_javascript, brd, null)' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'new CSLProcessorOutputConsumer(BASE_PATH, citations_javascript, RefreshDocument_OnBibliographyReady, passthru)' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'new GoogleBibTexSnifferControl()' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'new MainWindow()' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'new PDFPrinterDocumentPaginator(pdf_document, pdf_renderer, page_from, page_to, new Size(print_dialog.PrintableAreaWidth, print_dialog.PrintableAreaHeight))' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'new ReportViewerControl(annotation_report)' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'new StreamListenerTee()' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'new StreamWriter(client)' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'OpenWebBrowser()' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'this.OpenNewWindow(WebsiteAccess.Url_BlankWebsite)' before all references to it are out of scope.
warning CA2000: Call System.IDisposable.Dispose on object created by 'wbhc.OpenNewWindow()' before all references to it are out of scope.
warning CA2100: Review if the query string passed to 'SQLiteCommand.SQLiteCommand(string commandText, SQLiteConnection connection)' in 'GetIntranetLibraryItems', accepts any user input.
warning CA2100: Review if the query string passed to 'SQLiteCommand.SQLiteCommand(string commandText, SQLiteConnection connection)' in 'GetLibraryItems', accepts any user input.
warning CA2213: 'WebBrowserHostControl' contains field 'current_library' that is of IDisposable type 'Library', but it is never disposed. Change the Dispose method on 'WebBrowserHostControl' to call Close or Dispose on this field.
warning CA2234: Modify 'GoogleBibTexSnifferControl.PostBibTeXToAggregator(string)' to call 'WebRequest.Create(Uri)' instead of 'WebRequest.Create(string)'.
warning CA2234: Modify 'ImportingIntoLibrary.AddNewDocumentToLibraryFromInternet_SYNCHRONOUS(Library, string)' to call 'WebRequest.Create(Uri)' instead of 'WebRequest.Create(string)'.
warning CA2237: Add [Serializable] to LocaleTable as this type implements ISerializable
warning CA2237: Add [Serializable] to SynchronisationStates as this type implements ISerializable

## These are A-Okay and VERY MUCH INTENTIONAL: ##

warning CA5359: The ServerCertificateValidationCallback is set to a function that accepts any server certificate, by always returning true. Ensure that server certificates are validated to verify the identity of the server receiving requests.
warning CA5364: Hard-coded use of deprecated security protocol Ssl3
warning CA5364: Hard-coded use of deprecated security protocol Tls
warning CA5364: Hard-coded use of deprecated security protocol Tls11
@GerHobbelt
Copy link
Collaborator Author

Fixed as per today (marker commit: 95dff9b), will be available in upcoming pre-release v82pre4.

Any further BibTeX editing hassles should be filed in a new commit and possibly reference this one for a history breadcrumbs trail.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛bug Something isn't working 🕵TLC Needs some special attention
Projects
None yet
Development

No branches or pull requests

1 participant