Document with valid attachment may not be found#283
Document with valid attachment may not be found#283NeilWJames wants to merge 5 commits intowp-document-revisions:mainfrom
Conversation
Merge for wp-document-revisions
This comment was marked as outdated.
This comment was marked as outdated.
benbalter
left a comment
There was a problem hiding this comment.
I had two questions about the timestamp id attribute changes, but otherwise, this looks ✨ . Thank you once again.
| // phpcs:disable WordPress.Security.EscapeOutput.OutputNotEscaped | ||
| // translators: %1$s is the post modified date in words, %2$s is the post modified date in time format, %3$s is how long ago the post was modified, %4$s is the author's name. | ||
| printf( __( 'Checked in <abbr class="timestamp" title="%1$s" id="%2$s">%3$s</abbr> ago by %4$s', 'wp-document-revisions' ), $mod_date, strtotime( $mod_date ), human_time_diff( (int) get_post_modified_time( 'U', true, $post->ID ), time() ), get_the_author_meta( 'display_name', $latest_version->post_author ) ); | ||
| printf( __( 'Checked in <abbr class="timestamp" title="%1$s" id="A%2$s">%3$s</abbr> ago by %4$s', 'wp-document-revisions' ), $mod_date, strtotime( $mod_date ), human_time_diff( (int) get_post_modified_time( 'U', true, $post->ID ), time() ), get_the_author_meta( 'display_name', $latest_version->post_author ) ); |
There was a problem hiding this comment.
Is this addition intentional?
There was a problem hiding this comment.
Oddly enough, yes, it was intentional. In the document metabox there is an id with the id of the modified date.
There was also the same id in the revision metabox (but see below).
I installed an a11y checking package that noted that the duplication.
This is to dis-ambiguate the two.
| } | ||
| ?> | ||
| <tr> | ||
| <td><a href="<?php echo esc_url( $fn ); ?>" title="<?php echo esc_attr( $revision->post_modified ); ?>" class="timestamp" id="<?php echo esc_attr( strtotime( $revision->post_modified ) ); ?>"><?php echo esc_html( human_time_diff( strtotime( $revision->post_modified_gmt ), time() ) ); ?></a></td> |
There was a problem hiding this comment.
Curious the motivation for this change here?
There was a problem hiding this comment.
In fact, I actually then thought of the id itself - and thought what is actually its purpose/usefulness (and could the document and its latest revision have the same id.
So I removed it in the revision metabox.
I should have removed the id attribute from the document metabox as well.
Happy to do that.
Neil
|
@NeilWJames Apologies if my repo tidying broke your PR. Did you still want to see these changes merged? I think they are valuable. Happy to help get the PR across the line if you are still interested. |
|
@benbalter, I do believe that these fixes are needed and should be released - even though I don't understand why I did not experience them. Clearly some users have been inconvienced. I decided that the best thing to do is to leave it alone for a couple of days for you to do whatever is needed - and then to resynchronise my fork with yours and make the changes again in a new PR. I have taken a copy to compare between them. I will then propose a new pull. P.S. I didn't realise until I explicitly looked for it just now that I had closed it. Shows the level of my understanding of Git processing. :( I saw that you had merged master into main. I just renamed it in my attempt to align the process. Possibly this deleted the PR. Oh, well. Regards, |
|
@NeilWJames I went ahead and resolved the merge conflicts that I introduced against your fork and merged the commits into |
Support forum user identified a valid document may give a Not Found error.
Whilst I don't understand why it worked for me, it was clearly a bug.
I believe that other users can also be experiencing this issue, so I have (hopefully) prepared for a 3.4.1 release.
There was also a user with permissions issue not showing a complete menu.
Whilst there was code to alert the user, in a multi-user environment, this may not have been seen, so I have modified the notification processing somewhat to ensure that the message is displayed.