Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Requesting changes to new implementation-blocked merging process #2055

Closed
cookiecrook opened this issue Oct 5, 2023 · 16 comments · Fixed by #2248
Closed

Requesting changes to new implementation-blocked merging process #2055

cookiecrook opened this issue Oct 5, 2023 · 16 comments · Fixed by #2248
Assignees

Comments

@cookiecrook
Copy link
Contributor

cookiecrook commented Oct 5, 2023

I added the Agenda label b/c I don't know how to make progress on PR #1860 with the new process expectation, which seems unintentionally circular.

I think the process now is effectively requiring that implementations write disparate, non-WPT implementation-specific automation tests, before ARIA merges its spec changes, which effectively blocks the WPT tests that would aide those implementations.

I appreciate that we've added processes, but I'm not sure what step is expected to happen next, the 4-year-old issue remains, and there seem to be circular dependencies that are preventing forward movement.

@cookiecrook
Copy link
Contributor Author

cookiecrook commented Oct 5, 2023

Instead of waiting to merge, perhaps the ARIA and AccName changes could merge with flags such as "waiting on implementations" and "waiting on tests"... Then we could use the spec change to write the tests, and use the tests to drive the implementations. Meanwhile, the editor's draft can point directly to the WPT tests and implementation bugs... When we get ready to publish, if those features haven't landed, they get pulled from the CR branch.

@cookiecrook
Copy link
Contributor Author

I recall @aleventhal may have raised a similar point of confusion at TPAC 2023 in Seville.

@spectranaut
Copy link
Contributor

spectranaut commented Oct 12, 2023

Well articulated problem thanks @cookiecrook :)

I'm pretty sure we are moving the ARIA spec to evergreen, before we do that, we definitely need to have a clear process in place. My personal goal is that anyone, an implementer or web author, should be able to read the editor's spec and know what is implemented or not implemented. The publications/snapshots are quickly to far behind the times.

When suggesting the original procedural change, I was influence by my experience in the javascript standard world, where normative changes do not land in the spec until tests and multiple implementations land. The reason for this is that the implementations often end up influencing the spec language itsself, so until it has been proven to be "implementable" and proven that the engines will do the work, the change doesn't land in the spec. But javascript is a lot more complicated that ARIA, so maybe the need for feedback from implementation experience isn't necessary. Especially since we seek implementation sign off in the spec in the committee meetings or on PRs.

In my opinion there are a couple options:

  1. Follow the ECMAScript model anyway:
    1. Write the spec prose, get sign off from the working group.
    2. When there is sign off (3 positive reviews):
      1. Write and land tests
      2. Open issues on browsers
    3. When there are multiple implementations/implementation commitments, merge into all relevant ARIA specifications.
  2. Or, land spec changes first (this model is a bit unclear to me):
    1. Write spec prose, get sign off from the working group.
    2. When there is sign off (3 positive reviews):
      1. Write tests, land them
      2. Open browser issues
      3. Merge the changes to the spec with message that it is not implemented yet, and link to tests and browser issues?
    3. When there are multiple implementations, remove the message on the feature that it is not yet implemented.

To me, honestly, the second option seems more complicated, because changes are often spread out over one or more specs, so I'm not sure where the message "this is not yet implemented" should be. Also, not all things are testable, so looking at tests is not quite enough at this point to know if something is implemented.

@cookiecrook
Copy link
Contributor Author

cookiecrook commented Oct 16, 2023

I'm not sure how either could solve the current problem. For example, I assume either of your options would end up as:

  1. the ARIA spec change is written and reviewed, waiting to be merged
  2. the AccNAme change (to be written) should point to a placeholder URL? And not wait for implementations, but define a new section of the AccName algorithm. Presumably this would also wait for review, but not merge?
  3. The WPT tests (also to be written) would point to both of the prior PRs as the source of truth, but land in WPT?
  4. The implementation bugs usually expect a feature to be in a viewable spec, and there would be none. ‡

‡ Many times browser engineers making the changes are not aware of WG internal process like this, so I think this process might delay or block implementation (due to tedium/confusion) potentially resulting in the individual engineer shifting their time priority to another bug or feature. This was the problem @aleventhal mentioned at TPAC 2023 in Sevilla.

In short, I think this spec process violates the HTML Design Principles, Priority of Consituencies:

In case of conflict, consider users over authors over implementors over specifiers over theoretical purity. In other words costs or difficulties to the user should be given more weight than costs to authors; which in turn should be given more weight than costs to implementors; which should be given more weight than costs to authors of the spec itself, which should be given more weight than those proposing changes for theoretical reasons alone. Of course, it is preferred to make things better for multiple constituencies at once.

Either of the process options would put greater cost on implementors than on the authors of the spec, so I'd strongly recommend a new option 3.

Land spec changes before any downstream dependency

  1. Write spec prose, get sign off from the working group.
  2. When there is sign off on the initial spec change:
    1. Merge the changes to the spec with message that it is not implemented yet
    2. Work on downstream dependencies:
      1. In this case, the blocked AccName change, and merge those changes to the AccName spec with message that it is not implemented yet
      2. Write the WPT tests (in this case, linking to the relevant portions of the ARIA and AccName specs)
    3. Write tests, land them, and open browser issues.
  3. File a new (editorial) PR linked the WPT tests from the relevant specs
  4. When there are multiple implementations, remove the message on the feature that it is not yet implemented.

@spectranaut
Copy link
Contributor

spectranaut commented Oct 16, 2023

Briefly, on the "Priority of Consituencies" point above: ultimately, process changes should serve users and web authors -- our process should help us evolve the web towards better accessibility and help us make a spec that is understandable to web authors. Both implementers and editors/authors of the spec have limited time, and the limited time of both groups can be a blocker for the evolution of accessibility on the web. I don't think the needs of the two groups are necessarily at odds, and I think we can create a process that helps us achieve our goals and is easy for both groups to follow.

Now onto "the 2 options", I really wrote only sketches of options originally, and @cookiecrook I think your process suggestion is a better and more thought out expansion of what I was aiming for in my original option #2, so when I refer to option #2, I mean yours.

A more sketch out option 1

In the editors call today, it was brought up that option 1 is not only close to what the javascript standard follows, but also close to what WHATWG follows. Here is WHATWG's working mode which describes the process.

See the process's requirements for a change to the spec:

Each normative change made to the standard needs to meet the following criteria:

  • It must have support from implementers.
  • It should have corresponding test changes, either in the form of new tests or modifications to existing tests.
  • Implementations bugs must be filed for each user agent that fails tests. (This is each user agent that doesn’t match the proposed changes. If the test changes are not adequate to reveal that, but it’s known through other means, the tests should be improved first.)
  • It should have been reviewed by one or more members of the community.

Here is an example of a PR with implementation bugs open, and here is an example of a PR that hasn't landed but WPT tests have landed.

Notice the requirement in WHATWG specifically for additions to the spec:

  • The addition must have the support of at least two implementers. (Such support is not binding in any way on the implementers, however.)

Additionally, the following are strongly recommended:

  • The support from implementers should be of the form “we would like to implement this soon” and not just “this seems like a reasonable idea”.
  • There should be no strong implementer objections to the new feature.
  • There should already be a prototype implementation or one being worked on side-by-side with the change to the standard.

For process option 1, I would suggested a slightly stricter set of requirements, because the goal of option 1 is to have an editors draft that only contains usable ARIA features.

But, in response to @cookiecrook's and @aleventhal's points of confusion about when something is "implementable" from an implementer's perspective, it seems like webkit and chrome can implement things from a PR :) Maybe it would help if we make it VERY CLEAR when something is ready for implementation, even to someone who is entirely new to working with the ARIA spec -- an "has consensus from working group" and "ready for implementation" label (we have a "waiting for implementation" but we could be more clear and more strict)? Or event at the top of PR? Or we always point to PR preview in the bugs we open?

With all that in mind, and @cookiecrook's problems with option 1 take into account, here is an attempt at a more filled out option 1.

  1. Write spec prose, get sign off from the working group.
  2. When there is sign off on the initial spec change:
    1. Work on downstream dependencies:
      1. Write the WPT tests, land immediately after reviewed (note 1)
      2. Write related spec changes (AccName and AAMs) (note 2)
      3. Make an issue on APG to get an "experimental" stage example
    2. Open bugs on browsers (note 3):
      1. If tests are possible and related spec changes necessary: Wait for the tests to land and the related spec changes to be approved, then open bugs on browsers.
      2. Otherwise: open bugs on browsers.
  3. When there is a least one implementation and two "priories to implement soon":
    1. Merge the changes to the ARIA spec
    2. Update links in dependent spec changes (aams and accname)
    3. If they have been implemented, merge dependent changes in other specs (aams and accname)
    4. If a dependent spec change has not been implemented yet (maybe this could happen with accname), wait for implementation before merging.
    5. Update the APG example to be no longer "experimental"

notes:

  • note 1: as for links, I think it is fine to link to the PR. The wpt popover tests link to the explain, for example. We could make a backlog issue to update the links when things land.
  • note 2: links from AAM and AccName PRs to new parts of the ARIA spec could be temporary links, whatever is useful for implementers looking at the PR to implement -- to be fixed when the PR on ARIA lands in step 3.
  • note 3: bugs on browser should refer to the PR and PR Preview, and maybe say quite clearly "this is ready for implementation and waiting for implementation to land in the spec". On the PR, we should make it very clear the spec has consensus from the working group and is ready for implementation.

option 2

The only concern we have from the editors meeting is how and where to make clear that a feature or change is not yet implemented, during this intermediate point, when the change involves multiple parts of the specification -- and whether this will make the spec more complicated to read.

I'd have to take a look at some normative PRs to get and idea of how this could be done, but if you already have suggestions, like from the nameFrom: heading PR even, I would be nice to look at.

@mcking65
Copy link
Contributor

I like option 1 because a PR clearly shows what has changed and thus what is not "ready for authors". If the editor's draft is sprinkled with notes about what is not ready for authors, that could make the spec very difficult to read.

For example, imagine we are working on change C1 to the grid role and change C2 to the tab role. Now imagine that C1 and C2 both need to change something in the prose or attribute table of aria-selected . How would we add information to the aria-selected section that can clearly communicate the state of C1 and C2? That sounds hard to do, and it could create a lot of confusion for authors and implementers alike.

Option 1 avoids all possibility for such confusion because C1 and C2 are in independent PRs.

@mcking65
Copy link
Contributor

Don't forget to include APG in the downstream dependencies. Related to this, we'll be discussing how to integrate experimental content into the APG tomorrow. See Add styles and index support for experimental content · Issue #2836 · w3c/aria-practices.

@spectranaut
Copy link
Contributor

good point @mcking65 ! I added APG to the option 1 process description.

@spectranaut
Copy link
Contributor

spectranaut commented Oct 31, 2023

Discussed in last week ARIA working group meeting: https://www.w3.org/2023/10/26-aria-minutes#t05

For those interested, please read the suggestion in James' comment here and my comment here - we will be discussing these options in the deep dive this week.

@cookiecrook
Copy link
Contributor Author

cookiecrook commented Nov 2, 2023

APG is downstream but not a dependency like the other issues. Still, it’s good to have it in the list as a reminder to file the trackers within the main process.

@cookiecrook
Copy link
Contributor Author

cookiecrook commented Nov 4, 2023

Discussed in the Oct 2 ARIA WG meeting... ARIA Editors are discussing moving the source of the main specs (ARIA, Core-AAM, AccName, and maybe HTML-AAM) back into the ARIA repo, which wouldn't solve everything, but would greatly reduce the complexities of implementing from cross-referencial PRs.

If they take this path forward, I think the separate issue trackers would remain, but PRs would all be against the ARIA repo.

@cookiecrook
Copy link
Contributor Author

cookiecrook commented Nov 6, 2023

@spectranaut note the PR Previews say:

Do not attempt to implement this version of the specification. Do not reference this version as authoritative in any way. Instead, see [Editor's draft.]

…so that should also be changed if the new expectation is to implement from the PR.

@spectranaut spectranaut self-assigned this Nov 28, 2023
@spectranaut
Copy link
Contributor

spectranaut commented Dec 12, 2023

We had an editors meeting to discuss the "mono repo" idea, here are the minutes: https://www.w3.org/2023/11/27-aria-editors-minutes.html#t02

@cookiecrook in particular, we want to bring up a concern we have:

If we have a "mono repo", the main benefit is that we have a single PR with all relevant and related specification changes. This will make it easier to keep all the specs in sync for a single feature, and will probably make it easier to author and review changes.

However, we are not sure how much easier it will be for implementers regarding one particular concern you raised: the links between specs in the "spec previews" of a "mono repo" will still be wrong. Like you pointed out is the case for the "spec previews" now, if you have a PR that has a change in ARIA and a corresponding change in AccName, the PR Previews of both changes point back to the editors spec, which does not have the correct change.

The same thing would happen in a mono repo, because the respec generates these cross-spec links, not "pre-preview". This is to say, we don't have a way to build these links temporarily point to the PR preview build specs. In pre-preview, the links that "work" are all relative links.

So, we'd like to know whether or not the "mono repo" still going to move us towards solving your concerns! If not let's continue the discussion -- because the work to go towards a mono repo is kind of huge and we only want to do it if it solves big enough problems.

Although personally, I think it still might benefit us to have a mono repo.

@spectranaut
Copy link
Contributor

Also I tried updating the process document: #2088

@cookiecrook
Copy link
Contributor Author

Even if the cross-spec links don't work, there's still utility in having related cross-spec changes go into a single PR.

@cookiecrook
Copy link
Contributor Author

@spectranaut wrote:

Although personally, I think it still might benefit us to have a mono repo.

I agree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants