Skip to content
New issue

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

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

Already on GitHub? # to your account

[Editing] Disable buttons until the first page is rendered #15295

Merged
merged 1 commit into from
Aug 9, 2022

Conversation

calixteman
Copy link
Contributor

No description provided.

@Snuffleupagus
Copy link
Collaborator

Sorry, but I don't understand how this patch actually implements what the commit says (i.e. "Disable buttons until the first page is rendered")?
If you actually want to do this, then I believe that you want to remove

pdf.js/web/base_viewer.js

Lines 730 to 734 in 589f72e

// Ensure that the Editor buttons, in the toolbar, are updated.
this.eventBus.dispatch("annotationeditormodechanged", {
source: this,
mode,
});
and instead add

if (this.#annotationEditorUIManager) {
  // Ensure that the Editor buttons, in the toolbar, are updated.
  this.eventBus.dispatch("annotationeditormodechanged", {
    source: this,
    mode: this.#annotationEditorMode,
  });
}

after the following line

@Snuffleupagus
Copy link
Collaborator

I don't believe that you need the changes in the web/toolbar.js file now, since the buttons should already be disabled by default and you now simply delay their enabling.

@calixteman
Copy link
Contributor Author

Strange...
It works as you expect it in Chrome but not in Firefox (with local server).
I added a

console.log("getViewerConfiguration", document.getElementById("editorFreeText").hasAttribute("disabled"))

in viewer.js::getViewerConfiguration and in Chrome I get a true when I get a false in Firefox.

So it isn't the most strange thing here...
I had a pending nightly update, hence I restart Firefox and everything is working fine now, I mean it works as expected.

@calixteman
Copy link
Contributor Author

And I've the same bug again....
console.log("getViewerConfiguration", document.getElementById("editorFreeText").hasAttribute("disabled")) is showing a false in Firefox.

@Snuffleupagus
Copy link
Collaborator

Snuffleupagus commented Aug 9, 2022

Isn't that just the soft- vs hard-reload thing again, i.e. F5 vs Ctrl + F5?
Hence that shouldn't affect the Firefox built-in viewer, at least I don't think so, and there's probably a bunch of other HTML-state in the viewer that behaves the same way already (so nothing that we need to special-case here as far as I'm concerned).

@calixteman
Copy link
Contributor Author

Yep it's exactly the issue I hit.

@Snuffleupagus
Copy link
Collaborator

/botio integrationtest

@pdfjsbot
Copy link

pdfjsbot commented Aug 9, 2022

From: Bot.io (Linux m4)


Received

Command cmd_integrationtest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.241.84.105:8877/495d3cf692111fe/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Aug 9, 2022

From: Bot.io (Windows)


Received

Command cmd_integrationtest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.193.163.58:8877/a701e93bc4d1a67/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Aug 9, 2022

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/495d3cf692111fe/output.txt

Total script time: 4.74 mins

  • Integration Tests: Passed

@pdfjsbot
Copy link

pdfjsbot commented Aug 9, 2022

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/a701e93bc4d1a67/output.txt

Total script time: 7.30 mins

  • Integration Tests: FAILED

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants