Most of these should fail as per my manual testing. All of these would have passed before. Now with the new routes I think it is debatable whether the add/edit/delete pages should have the tabs on them (they do have other tabs on the same level though), or not. But at least the edit page always should have the translate tab (and the translate page itself obviously).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gábor Hojtsy’s picture

Priority: Normal » Major
YesCT’s picture

Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, tab-regressions.patch, failed testing.

Gábor Hojtsy’s picture

Title: Tabs regressions » Core tab problems
Status: Needs work » Needs review
FileSize
890 bytes
1.71 KB
1.8 KB

So this is basically due to core being dumb again :D Eg. if you apply this patch most of the contact related ones will pass. The add/edit pages don't have the tabs anymore, but that is due to them being a separate route / path now, and it would be non-standard (impossible?) for the tabs to show up on them. They are on one level deeper vs. where the tabs show. So they'll have the breadcrumbs but not the tabs. The tests for those should go away from the patch, which combined with this core one would make it all pass.

Status: Needs review » Needs work

The last submitted patch, 4: core-tab-problems.patch, failed testing.

Gábor Hojtsy’s picture

FileSize
1.97 KB
578 bytes

Well, there are still two fails even with the core patch but that is due to missing permissions in the test.

Status: Needs review » Needs work

The last submitted patch, 6: core-tab-problems.patch, failed testing.

YesCT’s picture

Gábor Hojtsy’s picture

Title: Core tab problems » Expand test coverage on translation tabs
Status: Needs work » Postponed

So what we can do here is we get these two issues fixed in core:

- #2135787: Move static config entity local tasks to local_tasks.yml
- #2111823: Convert field_ui / Entity local tasks to YAML definitions

Those would provide the tabs in the *current* tab system instead of the *old* tab system, so we can extend them. Unless those are fixed, the tabs themselves will only work on some config entities.

Gábor Hojtsy’s picture

Project: (Obsolete) configuration translation for Drupal 8 » Drupal core
Version: 8.x-1.x-dev » 8.x-dev
Component: Code » config_translation.module
Issue tags: +D8MI, +language-config

Will need a reroll against core now, however still postponed on the other two patches.

Gábor Hojtsy’s picture

Title: Expand test coverage on translation tabs » Expand test coverage on configuration translation tabs

Fully qualify which translation tabs.

mgifford’s picture

Status: Postponed » Needs work
Related issues: +#2111823: Convert field_ui / Entity local tasks to YAML definitions
stefank’s picture

As the two issues seems to be fixed, rerolling the patch from #6.

stefank’s picture

Status: Needs work » Needs review

Triggering testbot.

Status: Needs review » Needs work

The last submitted patch, 13: core-tab-problems-2135101_13.patch, failed testing.

YesCT’s picture

@stefank can you verify manually that the translate tabs are missing on those pages? (or maybe they are named differently?)

stefank’s picture

Status: Needs work » Needs review
FileSize
1.98 KB

@YesCT The translate tab is there, but the name changed to "Translate contact form", also the settings changed to "account settings". Hopefully that should pass now (it does pass locally).
Thanks for pointing it out.

YesCT’s picture

FileSize
1.92 KB

to see what that change looked like, I made an interdiff.
For instructions on creating an interdiff, see https://drupal.org/documentation/git/interdiff

tstoeckler’s picture

Status: Needs review » Reviewed & tested by the community

Awesome, thanks!

Gábor Hojtsy’s picture

Issue tags: +sprint
alexpott’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: -sprint

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per #2350615: [policy, no patch] What changes can be accepted during the Drupal 8 beta phase?. Committed c7dca6d and pushed to 8.0.x. Thanks!

  • alexpott committed c7dca6d on 8.0.x
    Issue #2135101 by Gábor Hojtsy, stefank, YesCT: Fixed Expand test...

Status: Fixed » Closed (fixed)

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