Repository navigation
Documents: adversarial review fixes (round 7) - #136
Merged
Merged
Conversation
…ed paragraph A paragraph's formatting change spans the paragraph, so Next Change went on from its end and never reached the words typed inside it (it stayed on the formatting change), and Previous Change from one of those words never reached the paragraph's change. From a selected change, Next and Previous now step through the changes in order.
…ack into the document The comment column is rebuilt when the caret moves to another thread, and only the text box that had the keyboard kept its words: a reply or an edit being written was emptied by a click in the document. Unsent replies and edits are now kept per thread until they are posted or cancelled.
… thread resolved in Word Word marks only a resolved thread's first comment done. Deleting all resolved comments removed that comment alone, so its replies came back as new, open threads. Deleting resolved comments now deletes each resolved thread whole.
…ader goes to the change The entry selected the change in the body but left the header open for editing, so nothing happened. It now leaves the header, footer or note first, as Next Change and comment entries do.
…rack Changes Alt+Shift+Up/Down (Control+Shift on a Mac) reordered paragraphs untracked while Track Changes was on, so reviewers never saw the move and Reject All could not undo it. Like Sort and outline moves, it now says the move cannot be tracked.
… header is edited
Editing a header, footer or note, the status bar counted only that story's words ("2 words") while the Word Count dialog it opens counted the document's. It now counts the document's words, and words selected in the header against them.
A document saved with insertions, deletions or formatting changes hidden (revisionView) hid them, and the next document opened kept them hidden although its settings show them, as it did a choice made in Show Markup. Opening a document now applies its own settings either way.
…rs hidden in Show Markup With a reviewer hidden (Show Markup > Specific People), Next Change and Next Comment still stopped at that reviewer's changes and comments, selecting text with no mark or card to show for it. They now pass over what Show Markup hides.
Choosing a comment keeps its card level with its text and moves the cards before it up to make room. Near the top of the document there is no room, and those cards were all drawn at the top, one over another, so all but one could not be read or clicked. They now stack down from the top, and the chosen card moves down below them.
… being thrown away Posting a new comment, a reply or an edit that the document refused (a protected document, a comment over 32,000 characters) showed the error and discarded the words typed. They now stay in the draft, reply or edit box, focused, to post again or cancel.
…tion allows none In a document protected as read-only or for filling in forms (or opened read-only as its author recommends), New Comment, Reply, Resolve, Edit and Delete were offered, and what was written was refused only when posted. They are now unavailable there, and the New Comment shortcut says the document is protected.
…r comments, forms or reading only Under such protection Track Changes could still be turned on or off, changing the protected document's settings (and marking it unsaved) though nothing else could change. It is now unavailable there, as it is while tracking is locked.
…mention list The suggestions offered after typing @ were not tied to the comment box, and moving through them with the arrow keys changed only their look, so a screen reader said nothing. The box now controls the list and names the chosen person as its active descendant; the suggestions are no longer separate Tab stops.
…tting change alone With the caret in words typed into a paragraph that was also reformatted (both with Track Changes on), Accept or Reject resolved the words and the paragraph's formatting change around them: rejecting the words also undid the centering. They now act on the change the caret is in; the formatting change keeps its own balloon and entry.
No Markup and Original are meant to show the document without markup, but the comment cards and their tinted text stayed. They now hide the comments, as they hide changes and change bars, and Show Comments is unavailable there; going to a comment or starting one switches back to All Markup.
…w Markup hides With a reviewer, or insertions and deletions, hidden in Show Markup, the Reviewing Pane still listed and counted them, though the page and the margin showed none of them. It now lists what is shown.
Leaving the document (All Documents, an import, the Puter window's close button) flushed pending edits with an explicit save, so with AutoSave off an experiment the user meant to throw away replaced the file without a word. Leaving now asks: Save, Don't Save (the file stays as it was), or Cancel.
Autosave waited for a pause after the last edit (1.5 s, up to 30 s after slow saves), and every keystroke restarted the wait. Someone typing or dictating steadily for minutes got no save at all, and a crash or closed tab lost all of it. Edits now save at the latest half a minute after the first unsaved one.
The PDF text layer took only each line's text fragments; a list item's number or bullet is painted apart from them, so copying or searching an exported PDF gave "First step" for "1. First step" and numbered headings lost their numbers.
…exports
Exports removed every object character, so an equation ("E=mc2") and a symbol from Insert Symbol vanished from the exported text, though copying the same text keeps them. They now export as the text copying gives; a symbol in a symbol font's private range, which reads as an empty box without that font, and pictures still go.
A list numbered 1, 2, 1, 2 (Restart Numbering, or a number set part-way through) exported as one <ol> with no number on the restarted item, and as Markdown that every reader numbers 1, 2, 3, 4 (CommonMark takes a list's first number only). The web page now gives such an item its value, and Markdown starts a new list after a comment that ends the one before.
…rkdown exports A Markdown table cell cannot hold a list, and its items were written without their markers, so "1. Step one / 2. Step two" became "Step one<br>Step two". The items now keep their numbers and bullets as text, as the plain-text export does.
…y embedded in it Text copied from this editor carries a description of what was copied (data-documents-fragment), which a paste reads instead of the HTML. Opening a web page or Markdown file, or inserting one with Text from File, read it too: a page holding one (a copy pasted into a site that keeps data attributes, or a page made to show one text and open as another) opened as that description alone, dropping the rest of the page. Files are now read as they are.
…es saved in legacy code pages A text file that is not UTF-8 and reads as none of the Korean, Japanese or Chinese code pages was read as Windows-1252, so a Russian file saved by Notepad opened as "Ïðèâåò, ìèð". Windows-1251, 1253, 1255 and 1256 are now recognised when their letters make up the text beyond ASCII, stand in words of their own, and are mostly their languages' common letters; Western text still reads as Windows-1252.
…it in opened web pages A web page or pasted HTML link written href=" https://… " (or with a line break before the address) works in a browser, which leaves that whitespace out, but lost its link here: the address had to start with https:, mailto: or tel: as written.
Save as PDF wrote each page as a picture with a text layer and no link annotations, so web, mail and phone links in the document could not be followed in the PDF, as they can in one a word processor exports. Each line (or picture) a link covers is now a link annotation with its address.
…ding order
The PDF text layer held each run's characters in reading order, left to right from the run's start. PDF readers take a page's glyphs as they stand from left to right and put right-to-left text back in reading order, so Hebrew and Arabic copied from an exported PDF came out reversed ("םולש" for "שלום") and could not be found. The text layer now holds them in the order they are painted, with brackets mirrored and each letter's marks kept.
… LibreOffice and Google Docs A web page's table becomes one tab-separated paragraph per row. Word, LibreOffice and Google Docs put a paragraph inside every cell, and each of those started a paragraph of its own, so a two-column table opened (or pasted) as one cell per line and its rows were lost. A cell's paragraphs are now text of its row, one after another.
… numbered there Each list item of another application's HTML (and of Markdown, which opens through it) was made a list item on its own, joining the paragraph above when that was one. Steps with code or a second paragraph under each item (as READMEs write them) all came out as "1.", a list's start number was ignored, and two lists one after another ran on as one. Each list now numbers on through what its items hold, starts at its own number, and starts again after another list.
…uest's client claims The owner's tabs choose which of them writes the file by the presence entries that say they save. Any entry saying so from a session that sorted first made the owner's tab stop saving, though only an owner may write the file: a guest's modified client could stop the file being saved, leaving everyone's changes only in the room. Only entries the service stamps with the owner's role count now.
… editing A HYPERLINK field to a file, a relative path or a UNC path made the whole document read-only, although a w:hyperlink element to the same target is kept and editable. Such a field is now kept as written: the document opens for editing, the link shows but is never followed (the link card says it can't be opened here), and the field saves unchanged. javascript:, vbscript: and data: addresses still keep the document read-only, with a plain notice.
New and renamed bookmarks had to be named with ASCII letters and digits, so a name like Résumé, Закладка or 書籤, which word processors accept, was refused. Letters and digits of any script (with their combining marks) and underscores are now accepted; the name still starts with a letter, has no spaces or punctuation, and is at most 40 characters.
The Bookmarks dialog labelled each bookmark with its sheet's position (page 1) rather than the number the page shows (page ii in a section numbered from ii), and the list of bookmarks Go To and the dialogs read came from the story being edited, empty in a header, footer or note. The list now always holds the text's bookmarks with their printed page numbers; renaming or deleting one from a story works on the text.
…note and Endnote dialog Apply (and Insert) in the Footnote and Endnote dialog wrote every footnote and endnote setting, even when nothing had changed: an untouched document gained a settings part, and changing the footnote number format also added endnote settings. Only the settings that differ from the document's are now written.
…ide and Web Layout Cards were placed right of the widest page, as if pages were stacked: in Read Mode and Side to Side, where pages lie across, a card was cut off at the window's edge or covered the next page, and in Web Layout the text took the whole width so the cards fell outside it. Pages laid across now keep room for the cards after each reading screen (or each page side to side), the reading columns leave that room, each card sits beside the last page of its group, and Web Layout and Draft lay the text out beside the column.
…ves the whole move A move's two halves (moveFrom and moveTo, linked by their range name, ECMA-376 §17.13.5.23–.24) are one change, but Accept or Reject on the text moved to left the text it was moved from deleted and still pending, and the reverse. Resolving either half, at the caret, over a selection or from its balloon, now resolves both.
…de pane open The column beside the pages kept a fixed 288-pixel card and 330 pixels of room, so at about 1100 pixels wide, or with a pane such as Restrict Editing open, it ran 30 to 70 pixels past the window and the cards were cut off or needed sideways scrolling. Cards now narrow (down to 200 pixels) to the room beside the pages, and the room follows them.
The dark appearance's Dark page preference only changed a style variable used by a few dialog previews; the pages stayed white. While it is on, a screen page's canvas now turns every color it paints (paper, text, borders, shading, selection and caret) as an inverted page shows it, keeping hues, while pictures keep their own colors. Nothing is laid out again, and printing, PDF and saving paint their own sheets, which stay white.
…ay from the caret The status bar's Page X of Y always named the caret's page, so scrolling through a long document left it on page 1. While the caret's page is out of view it now names the page in the middle of the view, as desktop word processors do, and the caret's again at the next caret move or edit. It is worked out once a frame from the pages' offsets, with no layout work.
Go To counted pages from the first, so in a document whose sections restart or reformat their numbering, page 8 went to the eighth sheet rather than the page numbered 8. A page number now finds the first page that carries it, as it is shown too (iv, L, aa), p and s name a page in a section (p1s3), a number no page carries still counts from the first page, and + and − still move from the caret.
At the largest and smallest zoom the status bar's steps did nothing but looked ready. They are dimmed there and marked aria-disabled, and keep the keyboard when it stepped the zoom to the limit.
… auto-hidden Auto-hide adds a show-ribbon button to the title bar, which on a phone has no room to spare: the document name shrank to its first letter. There, the undo history list makes way for it (Undo itself stays), and the name keeps about the room it has otherwise.
In Simple Markup and No Markup tracked deletions are hidden (as is hidden text while it is not shown), but the caret still stopped at every one of their characters: arrows needed extra presses that moved nothing visible, Backspace after a hidden deletion stepped back into it without deleting anything, and what was typed next went into the deleted word. Arrows (by character or word, with or without Shift) now pass over hidden text whole, Up, Down, Home, End and clicks leave the caret beside it, Backspace and Delete remove the visible character beyond it, and typing never lands inside it. All Markup is unchanged.
…otes or content controls Whether copy and paste carried formatting was decided for the whole document: one footnote, field or content control anywhere made every copy and paste in it plain text. A copy now carries formatting unless the selection itself holds such content (as each run was already checked), and a paste takes formatting unless the paragraphs it goes into hold some.
A selection holding a table, with text around it (or the whole document), was copied as plain text with every cell on a line of its own, so pasting it into a spreadsheet or a plain-text field lost the table's shape. Cells of one row are now separated by tabs and rows by lines, as a block of cells already copied and as other word processors copy a table.
Undo and Redo restored the selection but always showed the caret on the forward side, so a caret that had been at the end of a wrapped line came back at the start of the next one, and Home or Up then worked from there. The caret's side (and page) is now kept with each undo step and restored with its selection.
…racters A word holding an optional hyphen (as Manual Hyphenation puts in), a zero-width joiner or a zero-width space was checked with it, so a correct word such as hyph-enation was marked misspelled. Those invisible characters are now left out of the word checked (and of Ignore All and Add to Dictionary), while a correction still replaces the whole word, keeping a possessive ending.
… instead of adding a second one Design > Watermark only knew the drawing it writes itself. In a document with a Word watermark (VML in w:pict), choosing a watermark added a second one on top of Word's, and Remove Watermark left Word's in place. Both now take Word's watermark out of the headers too, keeping the headers' own text.
…mark The Watermark gallery and Custom Watermark dialog read only the watermark this editor writes, so with a Word watermark they claimed there was none and Remove Watermark was unavailable. A Word text watermark now reads as its text, font, colour, see-through and layout, and a picture one as its picture and washout; the dialog also offers a font outside its own list, so OK keeps it.
…nd iPad The Draw tab paints the stroke being drawn on a canvas over the page, sized at the display's full pixel ratio. A page at 200% on a 3x display needs more than Safari's 4096 x 4096 limit, so that canvas would draw nothing. It now takes its size from canvas-backing.js, as the page canvas does, and paints at the resulting scale, so the ink still lands under the pointer.
Adding a watermark to a page-numbered header gives it two sibling building blocks, Page Numbers and Watermarks. Their w:id values can repeat, within the header or in a copied header in another part, and the control reader refused the repeated id, so the whole header fell back to read-only display. Header and footer building-block wrappers keep their id in the retained wrapper and are identified by nothing else, so a repeated id no longer stops editing; editing writes both wrappers back around their own paragraphs, and an unedited header saves unchanged.
…ing edited Update Fields pressed in a header, footer or note closed it, loaded the field modules, updated the body and only then reopened the story, so keys typed in that moment went into the body text (and the caret was put back where F9 was pressed, reordering what had been typed). The modules now load first, and closing the story, updating the body's fields and citations in one step, and reopening it with its current caret happen without a pause in between.
… applied and undone in one step Clear All Formatting, Replace All and Row height no longer roll back when a repeated header row grows too tall to leave its page room for the table body; the row is laid out once without repeating. The specs now assert that: no error, text and selection kept, pages added, and one undo restoring the height, pagination and selection.
…tarted from a header Since Find searches the main document (the body) also while a header is being edited, choosing Main document after replacing in the headers selects a body match and leaves the header. The spec then clicked Close Header and Footer, which was no longer there, with no time limit, and waited until the test timed out. It now checks that the main document's three matches are found in the body and the header is left, and goes on to the selection scope as before.
…es late On Windows and Linux the browser always pastes for Ctrl+Shift+V, but its paste event can arrive after the zero-delay timer that read the clipboard for chords that paste nothing, so the text came in twice. That timer now runs only on a Mac, where a browser paste that still follows it is dropped until the next key press.
…ide enough for them in tests On Linux fonts the Insert tab folds its Text group into one button at the default test window size, so two new specs could not find Quick Parts and Object. They now use the 1440 x 900 window other Insert tab specs use.
… Text mode scrolling test The test pressed Ctrl+Shift+H off a Mac, which is not Find and Replace there, so the dialog never opened on Linux.
It opens a fresh editor at four widths and waits for an autosave in each, which can take longer than the default 30 seconds on a busy machine.
Off a Mac, Chrome's own key binding for the chord already pastes and matches style, so also sending that command through the debugging protocol pasted twice on Linux.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adversarial review of the whole Documents app (round 7). Eight reviewers each covered one area: text input, layout and painting, DOCX reading and writing, tables and objects, references and fields, review (comments and Track Changes), the app shell and UI, and saving, collaboration and other formats. Each then did a second pass on what they had found. Where a fix depended on another area, it was built on the merged branch, so fixes that interact were tested together.
Every issue was reproduced before it was fixed, with a unit test or browser spec that fails on
main. Each commit holds one fix and its test (186 commits).Most important fixes
Lost work
Files that opened read-only now open for editing
w:firstLineCharsand related). These are in nearly every Chinese or WPS document.PowerPlusWaterMarkObjectand picture watermarks). They are now drawn, kept byte for byte, and understood by Design › Watermark.docPartObjblocks), including ones that also hold a watermark.nonein stylesPAGE \* ARABICfrom LibreOfficedocument2.xmlTrack Changes and protection
Layout and display
References
Input, clipboard and Find
Inc.,C++and$100. Find and Go To work from a header or footer.Tables and objects
Import and export
Shell and accessibility
Behaviour changes to note
docs/conformance.md:Testing
TZ=UTCand on Node 22 (CI's version).npm run build, including the license check, passes. Bundle size is within 0.2% ofmain.mainhad 123 failures, mostly pixel-reference specs that differ on this machine.main's list was rerun on both. Seven failed only on this branch: one spec that went on waiting for a Close Header button that no longer appears, and three specs per heading-row change that asserted the old behaviour. Each was updated after checking the new behaviour in its flow.main.maintoo, so the first run was compared test by test. Five tests failed only on this branch:maindoes not also have (132 vs 133).main, because their test inputs carry content kept untouched.check:conformancewith no arguments stops at an unrelated spec'sedited.docx. Many specs save that generic name, and the tool checks files by name. This also happens onmain.Known gaps left open
docs/collaboration.md.w:trPr/w:del,cellDel,cellMerge) are not written yet, so those edits are refused while tracking.core.xmlbyte for byte.