-
-
Notifications
You must be signed in to change notification settings - Fork 65
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
v82pre3: curious loss of BibTeX metadata when editing in both BibTeX popup and PDF view metadata side panel #121
Labels
🐛bug
Something isn't working
🕵code review
When the issue popped up due to code review or when (larger) code review is required.
🕵investigate
Needs further analysis to find the root cause.
🕵TLC
Needs some special attention
Milestone
Comments
GerHobbelt
added
🐛bug
Something isn't working
🕵investigate
Needs further analysis to find the root cause.
🕵code review
When the issue popped up due to code review or when (larger) code review is required.
🕵TLC
Needs some special attention
labels
Oct 24, 2019
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
Fixed as per today (marker commit: 95dff9b) via the referenced commit above. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
🐛bug
Something isn't working
🕵code review
When the issue popped up due to code review or when (larger) code review is required.
🕵investigate
Needs further analysis to find the root cause.
🕵TLC
Needs some special attention
Tough to reproduce (looks like some weird timing problem) but the popup BibTeX editor and BibTex side panel used on the same PDF document can sometimes loose the entire BibTeX record.
It starts with the observation that both editors are not consistently synchronized while editing the BibTeX, particularly when you enter invalid BibTeX or BibTeX with comment lines included but no or very minor actual BibTeX info, e.g. many lines with 'x' (comment) and a bibtex record that itself reads as
@article{x, title={x}}
or something along those lines -- was just testing editor control behaviour...The text was updated successfully, but these errors were encountered: