Problem/Motivation

Now that entity translation is in, it it's a bit of a pain to have to edit something (or view it) and then click on the translation tab. It would be nice to be able to go to the translation overview right from listings.

Proposed resolution

Include translation operation in overviews for translatable entities

Remaining tasks

  • (done) ui review. @Bojhan agrees to premise in #2 via irc
  • (done) first patch. @Lukas von Blarer in #9
  • (done) follow-up patch with final approach by @ancamp in #12, plus further followup rerolls
  • (done) reviews by @Schnitzel @Gábor Hojtsy @attiks @YesCT @plach
  • (done) screenshots by @YesCT in #20 and #58
  • (done) tests by @Schnitzel in #23
  • (done) showing just test fail by @nagwani in #26, and later again in #42

User interface changes

See #20 and #58
add-s02-with_trans-2012-11-21_1839.png

API changes

done.

Files: 
CommentFileSizeAuthor
#58 add-s01-notrans-2012-11-21_1836.png78.48 KBYesCT
#58 add-s02-with_trans-2012-11-21_1839.png85.19 KBYesCT
#57 interdiff-42-47.txt981 bytesYesCT
#57 history.txt1.54 KBYesCT
#55 drupal-add_translation_links_to_overviews-1832778-54.patch11.19 KBSchnitzel
PASSED: [[SimpleTest]]: [MySQL] 48,572 pass(es).
[ View ]
#54 interdiff-42-54.txt600 bytesSchnitzel
#47 drupal-add_translation_links_to_overviews-1832778-47.patch11.61 KBrvilar
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to enable simpletest module.
[ View ]
#42 drupal-add_translation_links_to_overviews-1832778-42.patch11.19 KBYesCT
PASSED: [[SimpleTest]]: [MySQL] 48,132 pass(es).
[ View ]
#42 drupal-add_translation_links_to_overviews-1832778-42-justtests.patch6.87 KBYesCT
FAILED: [[SimpleTest]]: [MySQL] 48,132 pass(es), 4 fail(s), and 0 exception(s).
[ View ]
#42 interdiff-37-42.txt5 KBYesCT
#42 drupal-add_translation_links_to_overviews-1832778-42-cleanupAndFix.txt4.32 KBYesCT
#37 drupal-add_translation_links_to_overviews-1832778-37.patch11.21 KBSchnitzel
PASSED: [[SimpleTest]]: [MySQL] 48,093 pass(es).
[ View ]
#37 interdiff-23-37.txt6.16 KBSchnitzel
#26 interdiff.txt4.1 KBnagwani
#25 drupal-add_translation_links_to_overviews-1832778-23_fail.patch6.56 KBnagwani
FAILED: [[SimpleTest]]: [MySQL] 47,957 pass(es), 5 fail(s), and 0 exception(s).
[ View ]
#23 interdiff-15-23.txt6.56 KBSchnitzel
#23 drupal-add_translation_links_to_overviews-1832778-23.patch10.67 KBSchnitzel
PASSED: [[SimpleTest]]: [MySQL] 47,983 pass(es).
[ View ]
#20 transbutton-s01-nodelist-2012-11-07_0239.png72.42 KBYesCT
#20 transbutton-s02-on taxonomytermlist-2012-11-07_0255.png62.68 KBYesCT
#20 transbutton-s03-userlist-2012-11-07_0302.png60.13 KBYesCT
#20 transbutton-s04-commentlist-2012-11-07_0312.png90.85 KBYesCT
#16 drupal-add_translation_links_to_overviews-1832778-15.patch4.1 KBancamp
PASSED: [[SimpleTest]]: [MySQL] 47,820 pass(es).
[ View ]
#15 interdiff-12-15.txt3.92 KBancamp
#12 drupal-add_translation_links_to_overviews-1832778-12.patch3.29 KBancamp
FAILED: [[SimpleTest]]: [MySQL] 47,658 pass(es), 207 fail(s), and 94 exception(s).
[ View ]
#9 1832778-entity-overview-translation-operation-9.patch4.49 KBLukas von Blarer
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1832778-entity-overview-translation-operation-9.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
TranslateOP.jpg52.16 KBGábor Hojtsy

Comments

Gábor Hojtsy’s picture

I think the hard part about it will be that you need to check per entity on each row. The set of operations will possibly be different for different nodes based on whether they are translatable or not (have a base language, node type configured to be translatable, etc). Same for taxonomy terms, etc. I think the visibility/access rules for the menu tab would need to be applied essentially.

Gábor Hojtsy’s picture

@Bojhan agrees (via IRC).

Lukas von Blarer’s picture

I agree with that. Should I give it a try? Where can I start?

Gábor Hojtsy’s picture

@Lukas: I'd look into how are the node listing, taxonomy listed, user listing, comment listing, etc. are handled and add the translation option if the entity is translatable.

ancamp’s picture

Assigned:Unassigned» ancamp
ancamp’s picture

I have been looking into the code to figure out how to modify the list:
node.admin.inc
node.api.php
translation.module
translation_entity.module
I am still learning, and i will get back to it tomorrow!

attiks’s picture

For nodes have a look at node_admin_nodes inside node.admin.inc around line 550

Lukas von Blarer’s picture

Status:Active» Postponed

We should try to solve #1783964: Allow entity types to provide menu items before looking further into this.

Lukas von Blarer’s picture

StatusFileSize
new4.49 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1832778-entity-overview-translation-operation-9.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

I created a patch with ytsurk. We implemented a part of #1783964: Allow entity types to provide menu items to try to see if this approach makes sense. What do you guys think? Is this the right way to go?

ytsurk’s picture

the idea is using the getOperations of the EntityListController.
any entity can provide its own ListController.

if an operation needs to be added to a different entity, we introduced the hook_entity_operations.
there is a check missing so far, to recognize to what entity the operation should be added.
not sure if this would be the best solution ..

Gábor Hojtsy’s picture

Status:Postponed» Needs work

Erm, I don't think it would fly to add a list controller but not actually use it. These listings will most probably be implemented as VBO views in core. That is being proposed in #1828410: Provide a bulk_form element for actions. So implementing these as list controllers does not actually seem to advance towards a later goal either (which would maybe a possibility to argue for why we have them only minimally implemented and not much used).

So I think for now to make this work to improve usability tremendously, we want to just go and add the operations conditionally. The views/VBO conversion will later figure out how to convert this. We don't need to make this depend on anything.

ancamp’s picture

Assigned:ancamp» Unassigned
Status:Needs work» Needs review
StatusFileSize
new3.29 KB
FAILED: [[SimpleTest]]: [MySQL] 47,658 pass(es), 207 fail(s), and 94 exception(s).
[ View ]

I have added a translation link form the drop down OPERATIONS list in admin overviews if the entity (node/user/comment/taxonomy) is translatable.

YesCT’s picture

Gabor and I checked to see if TestEntity (test_entity) had a listing page to do the same thing there and there is no listing page for TestEntity, so that's ok.

Still needs a code review, a manual test and screenshots of the new screen.

Schnitzel’s picture

Status:Needs review» Needs work

did a Short review directly on the screen with @ancamp and found some minor changes.

ancamp’s picture

Status:Needs work» Needs review
StatusFileSize
new3.92 KB

Applied the changes given by Schnitzel.

ancamp’s picture

StatusFileSize
new4.1 KB
PASSED: [[SimpleTest]]: [MySQL] 47,820 pass(es).
[ View ]

uups, forgot to upload the patch.

attiks’s picture

Status:Needs review» Reviewed & tested by the community

Forgot to mark it as RTBC, good work

ytsurk’s picture

not a very generic solution - but definitely the intended behavior for the 4 included entities ..
or maybe - we wanted to go too far ;)

Lukas von Blarer’s picture

Great to see progress on this! ytsurk an me kind of wanted to demo the list controller to see how well it does work with content listings instead of settings listings. But let's in this case look at this later on.

YesCT’s picture

Here is what the lists look like with translate added to the dropbuttons.

Node list

transbutton-s01-nodelist-2012-11-07_0239.png

Term list

transbutton-s02-on taxonomytermlist-2012-11-07_0255.png

User list

transbutton-s03-userlist-2012-11-07_0302.png

Comment list

(Note: I could not find the comment list and had to have attiks tell me where it was. I didn't even know there was a comment list. It's nice!)
transbutton-s04-commentlist-2012-11-07_0312.png

webchick’s picture

Assigned:Unassigned» Gábor Hojtsy
Status:Reviewed & tested by the community» Needs review

I think Gábor wanted to look at this first.

Schnitzel’s picture

and I'm writing tests currently :)

Schnitzel’s picture

StatusFileSize
new10.67 KB
PASSED: [[SimpleTest]]: [MySQL] 47,983 pass(es).
[ View ]
new6.56 KB

Updated patch with Tests :)
Testing translation links for:
- Terms
- Users
- Nodes
- Comments
also testing that for bundles which are not translatable there are no translation links (expect User as there are no bundles)

YesCT’s picture

Issue tags:+Novice

There is more to do over all, but there is a small part of this that could be a novice task: uploading a version of the patch from #23 which is just the tests without the fix so the testbot can show it fails.

nagwani’s picture

StatusFileSize
new6.56 KB
FAILED: [[SimpleTest]]: [MySQL] 47,957 pass(es), 5 fail(s), and 0 exception(s).
[ View ]

Updated patch after removing fixes so the testbox can show it fails

nagwani’s picture

StatusFileSize
new4.1 KB

Adding interdiff

YesCT’s picture

Just a quick note of something. I did not look through the tests in detail.

+++ b/core/modules/comment/lib/Drupal/comment/Tests/CommentTranslationUITest.phpundefined
@@ -92,4 +95,21 @@ protected function getNewEntityValues($langcode) {
+  /**
+   * Tests translate link on comment content admin page.
+   */
+  function testTranslateLinkContentAdminPage() {

+++ b/core/modules/node/lib/Drupal/node/Tests/NodeTranslationUITest.phpundefined
@@ -60,6 +60,23 @@ function getTranslatorPermissions() {
   /**
+   * Tests translate link on content admin page.
+   */
+  function testTranslateLinkContentAdminPage() {

Both of these say ContentAdminPage. I think the one for comments should say

function testTranslateLinkContentCommentAdminPage()

or maybe just
function testTranslateLinkCommentAdminPage()

Status:Needs review» Needs work

The last submitted patch, drupal-add_translation_links_to_overviews-1832778-23_fail.patch, failed testing.

YesCT’s picture

Status:Needs work» Needs review

These are questions I had about the patch (fix only) from #16. I dont think anything is really wrong, but @ancamp if you can answer the questions, that would be nice; I'm curious.

+++ b/core/modules/comment/comment.admin.incundefined
@@ -136,6 +136,14 @@ function comment_admin_overview($form, &$form_state, $arg) {
+    if (module_invoke('translation_entity', 'enabled', 'comment', $comment->bundle())) {
+      $links['translate'] = array(
+        'title' => t('translate'),
+        'href' => 'comment/' . $comment->cid . '/translations',
+        'query' => $destination,
+      );
+    }
+
     $options[$comment->cid]['operations']['data'] = array(

this is a picky whitespace thing, but in the comment.admin.inc is the only time the blank line is added between the new block and the $options line that follows. The node, taxonomy and user do not have that blank line. So I would suggest either take out this blank. Or, also add it in the other files if the blank line is helping readability of the code you are adding in by separating it a bit from near code.

+++ b/core/modules/node/node.admin.incundefined
@@ -560,6 +560,13 @@ function node_admin_nodes() {
+    if (module_invoke('translation_entity', 'enabled', 'node', $node->bundle())) {
+      $operations['translate'] = array(

+++ b/core/modules/taxonomy/taxonomy.admin.incundefined
@@ -285,9 +285,16 @@ function taxonomy_overview_terms($form, &$form_state, Vocabulary $vocabulary) {
+    if (module_invoke('translation_entity', 'enabled', 'taxonomy_term', $term->bundle())) {
+      $operations['translate'] = array(

+++ b/core/modules/user/user.admin.incundefined
@@ -214,9 +215,16 @@ function user_admin_account() {
+    if ($translation_enabled) {
+      $links['translate'] = array(

comments and users are adding a links['translate'] array, and nodes and terms are adding a operations['translate'] array.
The screenshots in comment #20 show translate being added to each "OPERATIONS" column.

And why is user the only one to use $translation_enabled instead of module_invoke()? Maybe because users do not use a bundle?

+++ b/core/modules/taxonomy/taxonomy.admin.incundefined
@@ -285,9 +285,16 @@ function taxonomy_overview_terms($form, &$form_state, Vocabulary $vocabulary) {
-      'edit' => array('title' => t('edit'), 'href' => 'taxonomy/term/' . $term->tid . '/edit', 'query' => drupal_get_destination()),
-      'delete' => array('title' => t('delete'), 'href' => 'taxonomy/term/' . $term->tid . '/delete', 'query' => drupal_get_destination()),
+      'edit' => array('title' => t('edit'), 'href' => 'taxonomy/term/' . $term->tid . '/edit', 'query' => $destination),
+      'delete' => array('title' => t('delete'), 'href' => 'taxonomy/term/' . $term->tid . '/delete', 'query' => $destination),

why the change from drupal_get_destination() to destination()? Just to make it consistant with the other tests?

+++ b/core/modules/user/user.admin.incundefined
@@ -214,9 +215,16 @@ function user_admin_account() {
-      'href' => "user/$account->uid/edit",
+      'href' => 'user/' . $account->uid . '/edit',

this seems like an unrelated change.

Oh, I see. You are making it consistant with the single quote pattern used in the other $operations arrays.

I remember we talked about this at badcamp with Schnitzel and xjm. They said if it was very close to a block of code you were adding and related, that it made sense to go ahead and fix it.

Still at needs review because @webchick wanted to see what Gabor had to say in general about the fix or the tests. But I wonder if he already said it in #11.

Gábor Hojtsy’s picture

I think the main question and what was mentioned above as "patch is not too generic" is that we do these links in the modules themselves instead of making translation_entity.module do it for them. The reason for that is that translation_entity.module just cannot know about all the entity listings and their formats and where would the links go in each list and how. Eg. there are file entities, but no listing. So if a contrib module implements entity types, it would need a reproducible pattern from core. If core makes it look like translation_entity can do this for others on behalf of them, and it does not work for abitrary entity listings (how would it?), then that is a wrong message. So that is why I agree putting this into specific modules makes most sense.

Also, ideally the entity/content listings would be VBO views in core, so the whole thing we do here will be refactored to serve there and then the operations will move to some kind of operation method somewhere. They are not there now, and overloading this issue to add controllers and operations callbacks of all kinds I think would be too much, especially since we don't know how VBO is related.

So I think for the applicability to other contribs, this approach in the patch is well suited. It does not look elegant, but all other operations are also defined locally and introducing a custom operations override method would be overkill at this point, using form_alters would not be generic since the forms have slightly different structures.

YesCT’s picture

Assigned:Gábor Hojtsy» YesCT

I'll do two little things (the test name from #27 and the white space from #29 since I complained about them. :)

plach’s picture

Status:Needs work» Needs review

I agree with Gabor: for now we don't really have a cleaner solution for this. We might want to revisit the implementation once we have a more generic and unified API to build on. Meanwhile this is a nice UX improvement that it's good to have in.

+++ b/core/modules/comment/comment.admin.inc
@@ -136,6 +136,14 @@ function comment_admin_overview($form, &$form_state, $arg) {
+    if (module_invoke('translation_entity', 'enabled', 'comment', $comment->bundle())) {

+++ b/core/modules/node/node.admin.inc
@@ -560,6 +560,13 @@ function node_admin_nodes() {
+    if (module_invoke('translation_entity', 'enabled', 'node', $node->bundle())) {

+++ b/core/modules/taxonomy/taxonomy.admin.inc
@@ -285,9 +285,16 @@ function taxonomy_overview_terms($form, &$form_state, Vocabulary $vocabulary) {
     );
+    if (module_invoke('translation_entity', 'enabled', 'taxonomy_term', $term->bundle())) {

Using module_invoke() is a more elegant solution than what we are doing now in the regular comment display (we could fix that one too), however we should be invoking translation_entity_translate_access() instead, otherwise we aren't checking that the user is actually allowed to visit the translation overview. Currently clicking the link may result in a 403.

+++ b/core/modules/user/user.admin.inc
@@ -192,6 +192,7 @@ function user_admin_account() {
   $destination = drupal_get_destination();
+  $translation_enabled = module_invoke('translation_entity', 'enabled', 'user');

Same as above, but why we need a variable here instead of just putting the call in the if test? This would be more consistent.

plach’s picture

Status:Needs review» Needs work
Schnitzel’s picture

Status:Needs review» Needs work

I have some answers to @YesCT and @plach

@plach:

Same as above, but why we need a variable here instead of just putting the call in the if test? This would be more consistent.

performance, calling it once is much much faster then all the time for each User. As there are no bundles for Users you can only enable translations once for Users and we don't have to check it every time a user is shown.

however we should be invoking translation_entity_translate_access() instead, otherwise we aren't checking that the user is actually allowed to visit the translation overview. Currently clicking the link may result in a 403.

oh, awesome found :) @YesCT when you are changin all the things already could you do this directly also? If not assign to me later then I will do this

@YesCT

And why is user the only one to use $translation_enabled instead of module_invoke()? Maybe because users do not use a bundle?

exactly same question from plach, reason: performance.

why the change from drupal_get_destination() to destination()? Just to make it consistant with the other tests?

Performance too, the destination is the same for every node, so why loading it for every node.

plach’s picture

performance, calling it once is much much faster then all the time for each User. As there are no bundles for Users you can only enable translations once for Users and we don't have to check it every time a user is shown.

Sorry, I missed the loop :)

YesCT’s picture

Assigned:YesCT» Unassigned

Not going to get to this tonight. So unassigning it form myself. I'll check in on it when I can come back to it, but if someone else addresses the reviewer comments first, that's ok. :)

Schnitzel’s picture

Status:Needs work» Needs review
StatusFileSize
new6.16 KB
new11.21 KB
PASSED: [[SimpleTest]]: [MySQL] 48,093 pass(es).
[ View ]

- fixed things from YesCT in #27 and #29
- now using translation_entity_translate_access() as suggested in #32, which also means now that we have to run translation_entity_translate_access() for every single user. Als had to fix the tests as they did not have in: 'translate any entity'

Schnitzel’s picture

One thing I really hate to do:

+++ b/core/modules/user/user.admin.incundefined
@@ -192,11 +192,11 @@ function user_admin_account() {
+    $account = user_load($account->uid);

but if not $entity->entityType(); in translation_entity_translate_access() does not work.

We should definitely use full Entities in user_admin_account(), but to change all this would be too much of work, so I would suggest to create follow ups for this.

YesCT’s picture

Assigned:Unassigned» YesCT
Status:Needs review» Needs work
YesCT’s picture

Assigned:YesCT» Unassigned
Status:Needs work» Needs review

sorry. cross post.

YesCT’s picture

Assigned:Unassigned» YesCT

I'm reviewing this. Found some comment grammar nits I'll fix. I'll also post a patch and a just tests patch.

YesCT’s picture

StatusFileSize
new4.32 KB
new5 KB
new6.87 KB
FAILED: [[SimpleTest]]: [MySQL] 48,132 pass(es), 4 fail(s), and 0 exception(s).
[ View ]
new11.19 KB
PASSED: [[SimpleTest]]: [MySQL] 48,132 pass(es).
[ View ]

Here is the whole patch. (it passes locally). I have some questions about it... so I'll post a review of my own patch in a few.
There is also a tests only patch that should fail (it fails locally).
There is also an interdiff to 37: I added period to the end of // comment sentences, and blank likes to before the class closing that were removed when the tests for this were added, and I changed non_translatable to untranslatable to match what was used in the main et-ui issue.
There is also a just cleanup and fix file ... just for completeness in case someone wants to look at that.

YesCT’s picture

Assigned:YesCT» Unassigned
Status:Needs review» Needs work
+++ b/core/modules/node/lib/Drupal/node/Tests/NodeTranslationUITest.phpundefined
@@ -60,6 +60,23 @@ function getTranslatorPermissions() {
   /**
+   * Tests translate link on content admin page.
+   */
+  function testTranslateLinkContentAdminPage() {
+    $this->admin_user = $this->drupalCreateUser(array('access administration pages', 'access content overview', 'administer nodes', 'bypass node access'));
+    $this->drupalLogin($this->admin_user);
+
+    $page = $this->drupalCreateNode(array('type' => 'page'));
+    $article = $this->drupalCreateNode(array('type' => 'article'));
+
+    // Verify translation links.
+    $this->drupalGet('admin/content');
+    $this->assertResponse(200);
+    $this->assertLinkByHref('node/' . $article->nid . '/translations');
+    $this->assertNoLinkByHref('node/' . $page->nid . '/translations');
+  }

Just a question: Are pages by default in the et-ui tests not translatable and articles are translatable?

+++ b/core/modules/user/lib/Drupal/user/Tests/UserTranslationUITest.phpundefined
@@ -59,4 +59,19 @@ protected function getNewEntityValues($langcode) {
+  /**
+   * Tests translate link on user admin list.
+   */
+  function testTranslateLinkUserAdminPage() {
+    $this->admin_user = $this->drupalCreateUser(array('access administration pages', 'administer users', 'translate any entity'));
+    $this->drupalLogin($this->admin_user);
+
+    $uid = $this->createEntity(array('name' => $this->randomName()), $this->langcodes[0]);
+
+    // Verify translation links.
+    $this->drupalGet('admin/people');
+    $this->assertResponse(200);
+    $this->assertLinkByHref('user/' . $uid . '/translations');
+  }

Needs work because...

user is the only test added here that does not check that translations is not added when appropriate.

I think this is because users are either all translatable or all not. So, I think the test should also disable translation for users and test with assertNoLink that the translation link is not added to the user list.

Next

unassigning myself so @Schnitzel (or someone else) can respond or fix.

Schnitzel’s picture

Status:Needs work» Needs review

Just a question: Are pages by default in the et-ui tests not translatable and articles are translatable?

yes, Entity Translation UI Tests which are used define this by default (not drupal core default).

So, I think the test should also disable translation for users and test with assertNoLink that the translation link is not added to the user list.

Not my opinion: The Tests are testing if a new feature implemented (the translation links) is working correctly.
The Entity Translation UI Tests which are already in, also do not Test if Access is successfully denied if translation is not enabled to an Entity.
As we are here only adding links and not real functionality I would suggest that we make a follow up to create better test coverage for Entity Translation UI and add these Tests there.

back to needs review.

YesCT’s picture

Status:Needs review» Reviewed & tested by the community

given my questions were answered, RTBC.

Gábor Hojtsy’s picture

Status:Reviewed & tested by the community» Needs work
+++ b/core/modules/taxonomy/taxonomy.admin.incundefined
@@ -285,9 +285,16 @@ function taxonomy_overview_terms($form, &$form_state, Vocabulary $vocabulary) {
+        'query' => $destination,
+        );
+    };

Minor code style violation here. Looks good otherwise.

rvilar’s picture

Status:Needs work» Needs review
StatusFileSize
new11.61 KB
FAILED: [[SimpleTest]]: [MySQL] Failed to run tests: failed to enable simpletest module.
[ View ]

Here is a patch that fixes comment on #46

Status:Needs review» Needs work
Issue tags:-Usability, -Novice, -D8MI, -language-content

The last submitted patch, drupal-add_translation_links_to_overviews-1832778-47.patch, failed testing.

Gábor Hojtsy’s picture

Status:Needs work» Needs review

Status:Needs review» Needs work
Issue tags:+Usability, +Novice, +D8MI, +language-content

The last submitted patch, drupal-add_translation_links_to_overviews-1832778-47.patch, failed testing.

YesCT’s picture

@rvilar (or other) an interdiff between #42 and #47 might help.

rvilar’s picture

@YesCT I only remove the ';' character that Gabor shows in comment #46

Gábor Hojtsy’s picture

@rvilar: you know that is funny because I did not notice that even. I noticed another issue which is still not solved (see below). But wondering why would it not pass tests now :/

+++ b/core/modules/taxonomy/taxonomy.admin.incundefined
@@ -285,9 +285,16 @@ function taxonomy_overview_terms($form, &$form_state, Vocabulary $vocabulary) {
+        'href' => 'taxonomy/term/' . $term->tid . '/translations',
+        'query' => $destination,
+        );
+    }

The other code style problem was not solved, the indentation is still off on the closing parenthesis.

Schnitzel’s picture

Status:Needs work» Needs review
StatusFileSize
new600 bytes

recreated the patch of #42 and only changed the codestyle issues, see interdiff.

Schnitzel’s picture

StatusFileSize
new11.19 KB
PASSED: [[SimpleTest]]: [MySQL] 48,572 pass(es).
[ View ]
attiks’s picture

Status:Needs review» Reviewed & tested by the community
YesCT’s picture

StatusFileSize
new1.54 KB
new981 bytes
+++ b/.htaccessundefined
@@ -95,7 +95,7 @@ DirectoryIndex index.php index.html index.htm
-  # RewriteBase /
+  RewriteBase /

@rvilar Thanks a lot for jumping in here. It was a mystery why the bot had trouble. But, I think the RewriteBase might have been the cause.

(Now I had to roll back to Nov 10 to get the old patches to apply and I'm no git expert, so I might have made an error... but I tried to sort it out. See the history.)

YesCT’s picture

I'll update the summary too.

in #21 webchick asked for Gabor to look at this.
Gabor addressed the approach in #30
plach and Schnitzel had a nice look and back and forth
then Gabor looked again #46

So I think it's been looked at well.

I did another manual test just to make sure the latest stuff is good.
More screen shots from before are in #20
But here is a quick reshot with patch from 54, first without translation enabled and then with translation enabled.

add-s01-notrans-2012-11-21_1836.png

with translation enabled

add-s02-with_trans-2012-11-21_1839.png

YesCT’s picture

Issue summary:View changes

Updated issue summary to show the recent screenshot and use the issue summary template to make it easy for a committer to see what the status is.

Gábor Hojtsy’s picture

Woot, let's get this in :)

Dries’s picture

Status:Reviewed & tested by the community» Fixed

Looks good to me. Committed to 8.x. Thanks.

YesCT’s picture

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary:View changes

Updated issue summary clarify that no api changes were made.