fix: fieldDropdown.getText works in node - #9048

Merged
maribethb merged 2 commits into
RaspberryPiFoundation:rc/v12.0.0from
maribethb:htmlelement
May 14, 2025
Merged

fix: fieldDropdown.getText works in node#9048
maribethb merged 2 commits into
RaspberryPiFoundation:rc/v12.0.0from
maribethb:htmlelement

Conversation

@maribethb

Copy link
Copy Markdown
Contributor

The basics

The details

Resolves

Fixes#9035

Proposed Changes

Checks that HTMLElement is defined before trying to use it

Reason for Changes

Dropdown fields should work in node

Test Coverage

Added a node test to make sure this change fixes the CI problems we saw in samples

Documentation

I logged a warning for the case that would hit if:

  1. Someone uses a FieldDropdown with HTMLElement options
  2. They use node, haven't supplied an implementation of HTMLElement, and call getText.

Honestly the chances anyone does this and doesn't run into a number of other issues is relatively small, but hopefully the warning message helps them out if so.

If someone is using node with just regular text options, they don't need to supply an implementation of HTMLElement and won't see the warning, so this doesn't affect the usual case (and before v12, the only case, as it wasn't possible to use element options).

Additional Information

@maribethb
maribethb requested a review from a team as a code ownerMay 14, 2025 17:51
@maribethb
maribethb requested a review from cpcallenMay 14, 2025 17:51
@google-cla

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@maribethb
maribethb changed the base branch from develop to rc/v12.0.0May 14, 2025 17:52
@maribethbmaribethb changed the title Htmlelementfix: fieldDropdown.getText works in nodeMay 14, 2025
@maribethbmaribethb added the PR: fix Fixes a bug label May 14, 2025
@github-actionsgithub-actionsBot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 14, 2025
@maribethb

Copy link
Copy Markdown
ContributorAuthor

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

image

@BenHenningBenHenning left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @maribethb! Approving to unblock, but had some thoughts (most of which likely don't need to happen now for v12).


assert.deepEqual(jsonAfter, json);
});
test('Dropdown getText works with no HTMLElement defined', function () {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to check: this test fails without the fix in place, right?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Comment threadcore/field_dropdown.ts Outdated
"you're using HTMLElement dropdown options in node, ensure you're " +
'using jsdom-global or similar.',
);
return null;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth verifying the console warning & null case in tests?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because in order to get here you'd have to pass something that is an HTMElement type but then don't have HTMLElement defined. It would be really annoying to set up in tests and is a scenario that is unlikely to happen at all. But I had to return null to satisfy the ts compiler and the warning helps figure out why that is, in the off chance it ever somehow does happen.

@maribethb
maribethb merged commit 523dca9 into RaspberryPiFoundation:rc/v12.0.0May 14, 2025
@maribethb
maribethb deleted the htmlelement branch July 7, 2026 20:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: fixFixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

can't call FieldDropdown.getText in node

3 participants

@maribethb@BenHenning@cpcallen
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix: fieldDropdown.getText works in node - #9048

Merged
maribethb merged 2 commits into
RaspberryPiFoundation:rc/v12.0.0from
maribethb:htmlelement
May 14, 2025
Merged

fix: fieldDropdown.getText works in node#9048
maribethb merged 2 commits into
RaspberryPiFoundation:rc/v12.0.0from
maribethb:htmlelement

Conversation

@maribethb

Copy link
Copy Markdown
Contributor

The basics

The details

Resolves

Fixes#9035

Proposed Changes

Checks that HTMLElement is defined before trying to use it

Reason for Changes

Dropdown fields should work in node

Test Coverage

Added a node test to make sure this change fixes the CI problems we saw in samples

Documentation

I logged a warning for the case that would hit if:

  1. Someone uses a FieldDropdown with HTMLElement options
  2. They use node, haven't supplied an implementation of HTMLElement, and call getText.

Honestly the chances anyone does this and doesn't run into a number of other issues is relatively small, but hopefully the warning message helps them out if so.

If someone is using node with just regular text options, they don't need to supply an implementation of HTMLElement and won't see the warning, so this doesn't affect the usual case (and before v12, the only case, as it wasn't possible to use element options).

Additional Information

@maribethb
maribethb requested a review from a team as a code ownerMay 14, 2025 17:51
@maribethb
maribethb requested a review from cpcallenMay 14, 2025 17:51
@google-cla

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@maribethb
maribethb changed the base branch from develop to rc/v12.0.0May 14, 2025 17:52
@maribethbmaribethb changed the title Htmlelementfix: fieldDropdown.getText works in nodeMay 14, 2025
@maribethbmaribethb added the PR: fix Fixes a bug label May 14, 2025
@github-actionsgithub-actionsBot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 14, 2025
@maribethb

Copy link
Copy Markdown
ContributorAuthor

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

image

@BenHenningBenHenning left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @maribethb! Approving to unblock, but had some thoughts (most of which likely don't need to happen now for v12).


assert.deepEqual(jsonAfter, json);
});
test('Dropdown getText works with no HTMLElement defined', function () {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to check: this test fails without the fix in place, right?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Comment threadcore/field_dropdown.ts Outdated
"you're using HTMLElement dropdown options in node, ensure you're " +
'using jsdom-global or similar.',
);
return null;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth verifying the console warning & null case in tests?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because in order to get here you'd have to pass something that is an HTMElement type but then don't have HTMLElement defined. It would be really annoying to set up in tests and is a scenario that is unlikely to happen at all. But I had to return null to satisfy the ts compiler and the warning helps figure out why that is, in the off chance it ever somehow does happen.

@maribethb
maribethb merged commit 523dca9 into RaspberryPiFoundation:rc/v12.0.0May 14, 2025
@maribethb
maribethb deleted the htmlelement branch July 7, 2026 20:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: fixFixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

can't call FieldDropdown.getText in node

3 participants

@maribethb@BenHenning@cpcallen
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: fieldDropdown.getText works in node - #9048

Merged
maribethb merged 2 commits into
RaspberryPiFoundation:rc/v12.0.0from
maribethb:htmlelement
May 14, 2025
Merged

fix: fieldDropdown.getText works in node#9048
maribethb merged 2 commits into
RaspberryPiFoundation:rc/v12.0.0from
maribethb:htmlelement

Conversation

@maribethb

Copy link
Copy Markdown
Contributor

The basics

The details

Resolves

Fixes#9035

Proposed Changes

Checks that HTMLElement is defined before trying to use it

Reason for Changes

Dropdown fields should work in node

Test Coverage

Added a node test to make sure this change fixes the CI problems we saw in samples

Documentation

I logged a warning for the case that would hit if:

  1. Someone uses a FieldDropdown with HTMLElement options
  2. They use node, haven't supplied an implementation of HTMLElement, and call getText.

Honestly the chances anyone does this and doesn't run into a number of other issues is relatively small, but hopefully the warning message helps them out if so.

If someone is using node with just regular text options, they don't need to supply an implementation of HTMLElement and won't see the warning, so this doesn't affect the usual case (and before v12, the only case, as it wasn't possible to use element options).

Additional Information

@maribethb
maribethb requested a review from a team as a code ownerMay 14, 2025 17:51
@maribethb
maribethb requested a review from cpcallenMay 14, 2025 17:51
@google-cla

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@maribethb
maribethb changed the base branch from develop to rc/v12.0.0May 14, 2025 17:52
@maribethbmaribethb changed the title Htmlelementfix: fieldDropdown.getText works in nodeMay 14, 2025
@maribethbmaribethb added the PR: fix Fixes a bug label May 14, 2025
@github-actionsgithub-actionsBot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 14, 2025
@maribethb

Copy link
Copy Markdown
ContributorAuthor

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

image

@BenHenningBenHenning left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @maribethb! Approving to unblock, but had some thoughts (most of which likely don't need to happen now for v12).


assert.deepEqual(jsonAfter, json);
});
test('Dropdown getText works with no HTMLElement defined', function () {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to check: this test fails without the fix in place, right?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Comment threadcore/field_dropdown.ts Outdated
"you're using HTMLElement dropdown options in node, ensure you're " +
'using jsdom-global or similar.',
);
return null;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth verifying the console warning & null case in tests?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because in order to get here you'd have to pass something that is an HTMElement type but then don't have HTMLElement defined. It would be really annoying to set up in tests and is a scenario that is unlikely to happen at all. But I had to return null to satisfy the ts compiler and the warning helps figure out why that is, in the off chance it ever somehow does happen.

@maribethb
maribethb merged commit 523dca9 into RaspberryPiFoundation:rc/v12.0.0May 14, 2025
@maribethb
maribethb deleted the htmlelement branch July 7, 2026 20:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: fixFixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

can't call FieldDropdown.getText in node

3 participants

@maribethb@BenHenning@cpcallen
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: fieldDropdown.getText works in node - #9048

Merged
maribethb merged 2 commits into
RaspberryPiFoundation:rc/v12.0.0from
maribethb:htmlelement
May 14, 2025
Merged

fix: fieldDropdown.getText works in node#9048
maribethb merged 2 commits into
RaspberryPiFoundation:rc/v12.0.0from
maribethb:htmlelement

Conversation

@maribethb

Copy link
Copy Markdown
Contributor

The basics

The details

Resolves

Fixes#9035

Proposed Changes

Checks that HTMLElement is defined before trying to use it

Reason for Changes

Dropdown fields should work in node

Test Coverage

Added a node test to make sure this change fixes the CI problems we saw in samples

Documentation

I logged a warning for the case that would hit if:

  1. Someone uses a FieldDropdown with HTMLElement options
  2. They use node, haven't supplied an implementation of HTMLElement, and call getText.

Honestly the chances anyone does this and doesn't run into a number of other issues is relatively small, but hopefully the warning message helps them out if so.

If someone is using node with just regular text options, they don't need to supply an implementation of HTMLElement and won't see the warning, so this doesn't affect the usual case (and before v12, the only case, as it wasn't possible to use element options).

Additional Information

@maribethb
maribethb requested a review from a team as a code ownerMay 14, 2025 17:51
@maribethb
maribethb requested a review from cpcallenMay 14, 2025 17:51
@google-cla

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@maribethb
maribethb changed the base branch from develop to rc/v12.0.0May 14, 2025 17:52
@maribethbmaribethb changed the title Htmlelementfix: fieldDropdown.getText works in nodeMay 14, 2025
@maribethbmaribethb added the PR: fix Fixes a bug label May 14, 2025
@github-actionsgithub-actionsBot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 14, 2025
@maribethb

Copy link
Copy Markdown
ContributorAuthor

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

image

@BenHenningBenHenning left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @maribethb! Approving to unblock, but had some thoughts (most of which likely don't need to happen now for v12).


assert.deepEqual(jsonAfter, json);
});
test('Dropdown getText works with no HTMLElement defined', function () {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to check: this test fails without the fix in place, right?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Comment threadcore/field_dropdown.ts Outdated
"you're using HTMLElement dropdown options in node, ensure you're " +
'using jsdom-global or similar.',
);
return null;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth verifying the console warning & null case in tests?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because in order to get here you'd have to pass something that is an HTMElement type but then don't have HTMLElement defined. It would be really annoying to set up in tests and is a scenario that is unlikely to happen at all. But I had to return null to satisfy the ts compiler and the warning helps figure out why that is, in the off chance it ever somehow does happen.

@maribethb
maribethb merged commit 523dca9 into RaspberryPiFoundation:rc/v12.0.0May 14, 2025
@maribethb
maribethb deleted the htmlelement branch July 7, 2026 20:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: fixFixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

can't call FieldDropdown.getText in node

3 participants

@maribethb@BenHenning@cpcallen
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix: fieldDropdown.getText works in node - #9048

Merged
maribethb merged 2 commits into
RaspberryPiFoundation:rc/v12.0.0from
maribethb:htmlelement
May 14, 2025
Merged

fix: fieldDropdown.getText works in node#9048
maribethb merged 2 commits into
RaspberryPiFoundation:rc/v12.0.0from
maribethb:htmlelement

Conversation

@maribethb

Copy link
Copy Markdown
Contributor

The basics

The details

Resolves

Fixes#9035

Proposed Changes

Checks that HTMLElement is defined before trying to use it

Reason for Changes

Dropdown fields should work in node

Test Coverage

Added a node test to make sure this change fixes the CI problems we saw in samples

Documentation

I logged a warning for the case that would hit if:

  1. Someone uses a FieldDropdown with HTMLElement options
  2. They use node, haven't supplied an implementation of HTMLElement, and call getText.

Honestly the chances anyone does this and doesn't run into a number of other issues is relatively small, but hopefully the warning message helps them out if so.

If someone is using node with just regular text options, they don't need to supply an implementation of HTMLElement and won't see the warning, so this doesn't affect the usual case (and before v12, the only case, as it wasn't possible to use element options).

Additional Information

@maribethb
maribethb requested a review from a team as a code ownerMay 14, 2025 17:51
@maribethb
maribethb requested a review from cpcallenMay 14, 2025 17:51
@google-cla

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@maribethb
maribethb changed the base branch from develop to rc/v12.0.0May 14, 2025 17:52
@maribethbmaribethb changed the title Htmlelementfix: fieldDropdown.getText works in nodeMay 14, 2025
@maribethbmaribethb added the PR: fix Fixes a bug label May 14, 2025
@github-actionsgithub-actionsBot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 14, 2025
@maribethb

Copy link
Copy Markdown
ContributorAuthor

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

image

@BenHenningBenHenning left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @maribethb! Approving to unblock, but had some thoughts (most of which likely don't need to happen now for v12).


assert.deepEqual(jsonAfter, json);
});
test('Dropdown getText works with no HTMLElement defined', function () {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to check: this test fails without the fix in place, right?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Comment threadcore/field_dropdown.ts Outdated
"you're using HTMLElement dropdown options in node, ensure you're " +
'using jsdom-global or similar.',
);
return null;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth verifying the console warning & null case in tests?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because in order to get here you'd have to pass something that is an HTMElement type but then don't have HTMLElement defined. It would be really annoying to set up in tests and is a scenario that is unlikely to happen at all. But I had to return null to satisfy the ts compiler and the warning helps figure out why that is, in the off chance it ever somehow does happen.

@maribethb
maribethb merged commit 523dca9 into RaspberryPiFoundation:rc/v12.0.0May 14, 2025
@maribethb
maribethb deleted the htmlelement branch July 7, 2026 20:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: fixFixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

can't call FieldDropdown.getText in node

3 participants

@maribethb@BenHenning@cpcallen
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: fieldDropdown.getText works in node - #9048

Merged
maribethb merged 2 commits into
RaspberryPiFoundation:rc/v12.0.0from
maribethb:htmlelement
May 14, 2025
Merged

fix: fieldDropdown.getText works in node#9048
maribethb merged 2 commits into
RaspberryPiFoundation:rc/v12.0.0from
maribethb:htmlelement

Conversation

@maribethb

Copy link
Copy Markdown
Contributor

The basics

The details

Resolves

Fixes#9035

Proposed Changes

Checks that HTMLElement is defined before trying to use it

Reason for Changes

Dropdown fields should work in node

Test Coverage

Added a node test to make sure this change fixes the CI problems we saw in samples

Documentation

I logged a warning for the case that would hit if:

  1. Someone uses a FieldDropdown with HTMLElement options
  2. They use node, haven't supplied an implementation of HTMLElement, and call getText.

Honestly the chances anyone does this and doesn't run into a number of other issues is relatively small, but hopefully the warning message helps them out if so.

If someone is using node with just regular text options, they don't need to supply an implementation of HTMLElement and won't see the warning, so this doesn't affect the usual case (and before v12, the only case, as it wasn't possible to use element options).

Additional Information

@maribethb
maribethb requested a review from a team as a code ownerMay 14, 2025 17:51
@maribethb
maribethb requested a review from cpcallenMay 14, 2025 17:51
@google-cla

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@maribethb
maribethb changed the base branch from develop to rc/v12.0.0May 14, 2025 17:52
@maribethbmaribethb changed the title Htmlelementfix: fieldDropdown.getText works in nodeMay 14, 2025
@maribethbmaribethb added the PR: fix Fixes a bug label May 14, 2025
@github-actionsgithub-actionsBot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 14, 2025
@maribethb

Copy link
Copy Markdown
ContributorAuthor

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

image

@BenHenningBenHenning left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @maribethb! Approving to unblock, but had some thoughts (most of which likely don't need to happen now for v12).


assert.deepEqual(jsonAfter, json);
});
test('Dropdown getText works with no HTMLElement defined', function () {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to check: this test fails without the fix in place, right?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Comment threadcore/field_dropdown.ts Outdated
"you're using HTMLElement dropdown options in node, ensure you're " +
'using jsdom-global or similar.',
);
return null;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth verifying the console warning & null case in tests?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because in order to get here you'd have to pass something that is an HTMElement type but then don't have HTMLElement defined. It would be really annoying to set up in tests and is a scenario that is unlikely to happen at all. But I had to return null to satisfy the ts compiler and the warning helps figure out why that is, in the off chance it ever somehow does happen.

@maribethb
maribethb merged commit 523dca9 into RaspberryPiFoundation:rc/v12.0.0May 14, 2025
@maribethb
maribethb deleted the htmlelement branch July 7, 2026 20:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: fixFixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

can't call FieldDropdown.getText in node

3 participants

@maribethb@BenHenning@cpcallen
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: fieldDropdown.getText works in node - #9048

Merged
maribethb merged 2 commits into
RaspberryPiFoundation:rc/v12.0.0from
maribethb:htmlelement
May 14, 2025
Merged

fix: fieldDropdown.getText works in node#9048
maribethb merged 2 commits into
RaspberryPiFoundation:rc/v12.0.0from
maribethb:htmlelement

Conversation

@maribethb

Copy link
Copy Markdown
Contributor

The basics

The details

Resolves

Fixes#9035

Proposed Changes

Checks that HTMLElement is defined before trying to use it

Reason for Changes

Dropdown fields should work in node

Test Coverage

Added a node test to make sure this change fixes the CI problems we saw in samples

Documentation

I logged a warning for the case that would hit if:

  1. Someone uses a FieldDropdown with HTMLElement options
  2. They use node, haven't supplied an implementation of HTMLElement, and call getText.

Honestly the chances anyone does this and doesn't run into a number of other issues is relatively small, but hopefully the warning message helps them out if so.

If someone is using node with just regular text options, they don't need to supply an implementation of HTMLElement and won't see the warning, so this doesn't affect the usual case (and before v12, the only case, as it wasn't possible to use element options).

Additional Information

@maribethb
maribethb requested a review from a team as a code ownerMay 14, 2025 17:51
@maribethb
maribethb requested a review from cpcallenMay 14, 2025 17:51
@google-cla

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@maribethb
maribethb changed the base branch from develop to rc/v12.0.0May 14, 2025 17:52
@maribethbmaribethb changed the title Htmlelementfix: fieldDropdown.getText works in nodeMay 14, 2025
@maribethbmaribethb added the PR: fix Fixes a bug label May 14, 2025
@github-actionsgithub-actionsBot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 14, 2025
@maribethb

Copy link
Copy Markdown
ContributorAuthor

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

image

@BenHenningBenHenning left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @maribethb! Approving to unblock, but had some thoughts (most of which likely don't need to happen now for v12).


assert.deepEqual(jsonAfter, json);
});
test('Dropdown getText works with no HTMLElement defined', function () {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to check: this test fails without the fix in place, right?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Comment threadcore/field_dropdown.ts Outdated
"you're using HTMLElement dropdown options in node, ensure you're " +
'using jsdom-global or similar.',
);
return null;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth verifying the console warning & null case in tests?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because in order to get here you'd have to pass something that is an HTMElement type but then don't have HTMLElement defined. It would be really annoying to set up in tests and is a scenario that is unlikely to happen at all. But I had to return null to satisfy the ts compiler and the warning helps figure out why that is, in the off chance it ever somehow does happen.

@maribethb
maribethb merged commit 523dca9 into RaspberryPiFoundation:rc/v12.0.0May 14, 2025
@maribethb
maribethb deleted the htmlelement branch July 7, 2026 20:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: fixFixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

can't call FieldDropdown.getText in node

3 participants

@maribethb@BenHenning@cpcallen
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix: fieldDropdown.getText works in node - #9048

Merged
maribethb merged 2 commits into
RaspberryPiFoundation:rc/v12.0.0from
maribethb:htmlelement
May 14, 2025
Merged

fix: fieldDropdown.getText works in node#9048
maribethb merged 2 commits into
RaspberryPiFoundation:rc/v12.0.0from
maribethb:htmlelement

Conversation

@maribethb

Copy link
Copy Markdown
Contributor

The basics

The details

Resolves

Fixes#9035

Proposed Changes

Checks that HTMLElement is defined before trying to use it

Reason for Changes

Dropdown fields should work in node

Test Coverage

Added a node test to make sure this change fixes the CI problems we saw in samples

Documentation

I logged a warning for the case that would hit if:

  1. Someone uses a FieldDropdown with HTMLElement options
  2. They use node, haven't supplied an implementation of HTMLElement, and call getText.

Honestly the chances anyone does this and doesn't run into a number of other issues is relatively small, but hopefully the warning message helps them out if so.

If someone is using node with just regular text options, they don't need to supply an implementation of HTMLElement and won't see the warning, so this doesn't affect the usual case (and before v12, the only case, as it wasn't possible to use element options).

Additional Information

@maribethb
maribethb requested a review from a team as a code ownerMay 14, 2025 17:51
@maribethb
maribethb requested a review from cpcallenMay 14, 2025 17:51
@google-cla

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@maribethb
maribethb changed the base branch from develop to rc/v12.0.0May 14, 2025 17:52
@maribethbmaribethb changed the title Htmlelementfix: fieldDropdown.getText works in nodeMay 14, 2025
@maribethbmaribethb added the PR: fix Fixes a bug label May 14, 2025
@github-actionsgithub-actionsBot added PR: fix Fixes a bug and removed PR: fix Fixes a bug labels May 14, 2025
@maribethb

Copy link
Copy Markdown
ContributorAuthor

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

image

@BenHenningBenHenning left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @maribethb! Approving to unblock, but had some thoughts (most of which likely don't need to happen now for v12).


assert.deepEqual(jsonAfter, json);
});
test('Dropdown getText works with no HTMLElement defined', function () {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to check: this test fails without the fix in place, right?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Comment threadcore/field_dropdown.ts Outdated
"you're using HTMLElement dropdown options in node, ensure you're " +
'using jsdom-global or similar.',
);
return null;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it worth verifying the console warning & null case in tests?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because in order to get here you'd have to pass something that is an HTMElement type but then don't have HTMLElement defined. It would be really annoying to set up in tests and is a scenario that is unlikely to happen at all. But I had to return null to satisfy the ts compiler and the warning helps figure out why that is, in the off chance it ever somehow does happen.

@maribethb
maribethb merged commit 523dca9 into RaspberryPiFoundation:rc/v12.0.0May 14, 2025
@maribethb
maribethb deleted the htmlelement branch July 7, 2026 20:13
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: fixFixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

can't call FieldDropdown.getText in node

3 participants

@maribethb@BenHenning@cpcallen