OSSCA Chromium

기여 목록 / 8507314

Let the toolbar filter's regex toggle reflect programmatic state

DevToolsfixdevtools/devtools-frontendMerged

DevTools 툴바 필터의 정규식 토글이 코드로 설정한 상태를 반영하지 못하는 버그를 수정한 컨트리뷰션입니다. 직접 찾은 이슈이며, 지금까지의 정리 작업과 달리 실제 동작을 바꾸는 변경입니다.

문제 설명

  • 정규식 토글을 켠 상태에서 우클릭 View computed value를 쓰면 버튼은 켜진 채로 남지만 필터링은 일반 텍스트로 돌아감
  • 해당 파일 TODO에 알려진 문제로 기록되어 있었음
TODO(b/407751272): ... There is also a bug where the Toolbar Input's regex flag
cannot be controlled, so if you set a regex filter here, the toolbar might not reflect it.

StylePropertyTreeElement.viewComputedValue()가 정규식 필터를 적용한 뒤 속성명을 문자열로 대입하는데, filterText setter는 문자열을 받으면 #filterIsRegex를 false로 되돌립니다.

await computedStyleWidget.filterComputedStyles(regex);
computedStyleWidget.filterText = this.property.name;   // string

원인 추적 결과 버튼에 상태를 전달할 경로 자체가 없었습니다.

  • #updateView()가 뷰로 넘기는 입력에 #filterIsRegex가 없음
  • ToolbarFilter가 정규식 버튼을 생성자 지역 변수로만 보관
    • toggled: false로 고정 생성되고 이후 사용자 클릭으로만 변경
  • ToolbarInputElement.observedAttributes에도 토글 상태 항목이 없음

해결 내용

  1. ToolbarFilter가 버튼을 필드로 보관하고 setRegexToggled() 추가
  2. ToolbarInputElement에 regex-toggled 속성 추가
    • 기존 value / disabled와 동일하게 attributeChangedCallback에서 위임
    • 속성이 item 생성보다 먼저 설정된 경우도 반영
  3. ComputedStyleWidget이 #filterIsRegex를 뷰에 전달하고 템플릿에서 바인딩

기존 regex 속성은 상태가 아니라 버튼 표시 여부라서(생성자 인자명이 showRegexToggle) 별도 속성을 추가했습니다.

TODO는 해결된 후반부만 제거하고, 전반부(필터 관련 인스턴스 변수 3개 정리)는 동작 변경 없는 리팩터라 남겨두었습니다.

테스트 방법

  1. ui/legacy:ui, panels/elements:elements 및 각 :unittests 빌드
  2. Toolbar.test.ts에 테스트 3건 추가 (기본값 꺼짐 / setRegexToggled(true) 반영 / 토글 없을 때 무동작)
  3. Toolbar.test.ts + ComputedStyleWidget.test.ts → 36/36 통과

배운 점

  • 속성 이름만 보고 의미를 단정하면 안 됨
    • 처음에는 템플릿의 ?regex=${true} 하드코딩이 버그라고 봤으나, 생성자 인자명이 showRegexToggle이라 "버튼 표시 여부"였음
    • 하드코딩은 정상이었고 별도 속성이 필요한 상황이었음
  • 테스트 선택자도 구현 세부에 걸림
    • querySelector('devtools-button')이 clear 버튼을 먼저 잡았고, variant는 getter가 없어 jslogContext로 구분해야 했음
  • devtools presubmit은 자체 포매터 결과를 커밋하도록 요구
    • git cl format --dry-run이 통과해도 별개이며, 포매터 출력이 지저분하면 코드를 고쳐 깔끔한 출력이 나오게 하는 편이 나음

참고 자료

  • third_party/devtools-frontend/src/front_end/ui/legacy/Toolbar.ts
  • third_party/devtools-frontend/src/front_end/panels/elements/ComputedStyleWidget.ts

리뷰 대응

Danil Somsikov 님이 세 가지를 지적해 패치셋 2에서 반영했습니다.

  • 버그 설명이 틀렸음
    • 제가 쓴 재현 경로(filterText에 RegExp 주입)는 AI assistance만 쓰는 경로이고 그쪽은 툴바를 숨김
    • 사용자가 실제로 겪는 건 View computed value 경로라는 지적
  • 진실 공급원이 둘
    • 위젯이 #filterIsRegex를 맹목적으로 반전하는데 버튼도 스스로 토글하고 있었음
    • Lit은 마지막 렌더값과만 비교하므로 툴바가 렌더되지 않는 동안 두 값이 벌어질 수 있음
    • regextoggle 이벤트에 버튼 상태를 담아 반전이 아니라 대입하도록 변경 (ICON_TOGGLE은 click 리스너 실행 시점에 이미 자기 toggled를 바꿔둠)
  • 속성 경로 테스트 누락
    • 기존 3건은 setRegexToggled() 직접 호출 기준이었고 attributeChangedCallback 경로는 미검증
    • connect 전 설정 / connect 후 토글 / 제거 3건 추가

테스트 36건 → 40건. ComputedStyleWidget.test.ts에 buttons.js 임포트가 추가되어 panels/elements/BUILD.gn의 unittests 의존성도 함께 갱신했습니다.

  • 커밋 메시지 수정이 Gerrit 에 반영되지 않았던 문제
    • 패치셋 2 에서 메시지를 고쳤지만 git cl upload 가 Gerrit 설명에 반영하지 않아, Danil 님이 "Don't see that." 으로 지적
    • Gerrit 쪽 메시지를 직접 확인한 뒤 수정. 메시지만 바꾸면 Code-Review 표는 유지됨
  • 열린 코멘트 스레드가 있으면 머지가 불가능
    • Mathias Bynens 님이 해결된 스레드는 작성자가 닫으라고 안내. 닫은 직후 +1 과 CQ+2 로 머지