Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions global.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ declare global {
amdRequire: any
require: any
jQuery: JQueryStatic
innerShiv // TODO: For IE<9.. we could also remove it
}

//Jasmine and Mocha define duplicated functions, is a problem for the type system
Expand All @@ -30,7 +29,6 @@ declare global {
const undefined: undefined // Legacy Jasmine 1.x sentinel value

let browserSupportsProtoAssignment: any
let ieVersion: any
let updateInterval: number

interface Matchers<T> {
Expand Down
7 changes: 0 additions & 7 deletions packages/bind/spec/bindingAttributeBehaviors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -830,13 +830,6 @@ describe('Binding attribute syntax', function () {
return [Node.TEXT_NODE]
}
override nodeHasBindings(node) {
// IE < 9 can't bind text nodes, as expando properties are not allowed on them.
// This will still prove that the binding provider was not executed on the children of a restricted element.
if (node.nodeType === Node.TEXT_NODE && jasmine.ieVersion < 9) {
node.data = 'replaced'
return false
}

return true
}
override getBindingAccessors(node, bindingContext) {
Expand Down
10 changes: 1 addition & 9 deletions packages/binding.core/spec/hasfocusBehaviors.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { triggerEvent, registerEventHandler, ieVersion } from '@tko/utils'
import { triggerEvent, registerEventHandler } from '@tko/utils'

import { applyBindings, applyBindingsToNode } from '@tko/bind'

Expand Down Expand Up @@ -94,14 +94,6 @@ arrayForEach(['hasfocus', 'hasFocus', 'focusKnockout351'], binding => {
bindingHandlers.focusKnockout351 = legacyCustomBinding
})

if (ieVersion) {
// Workaround for spurious focus-timing-related failures on IE8
// (issue knockout/knockout#736)
beforeEach(function () {
waits(100)
})
}

it('Should set an observable value to be true on focus and false on blur even if the binding is applied through another binding', function () {
const createElementWithHasFocusBinding = {
init: (element: HTMLElement, valueAccessor: () => any) => {
Expand Down
47 changes: 32 additions & 15 deletions packages/binding.core/spec/textInputBehaviors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,35 @@ describe('Binding: TextInput', function () {
bindingHandlers.set(coreBindings)
})

it('User-Agent detection IE10 + IE11', function () {
let uaList = [
{ agent: 'Mozilla/5.0 (compatible; MSIE 9.0; Windows NT 6.1; WOW64; Trident/7.0', version: 9 },
{ agent: 'Mozilla/5.0 (compatible; MSIE 10.0; Windows NT 6.2; Trident/6.0)', version: 10 },
{ agent: 'Mozilla/5.0 (Windows NT 6.3; Trident/7.0; rv:11.0) like Gecko', version: 11 },
{
agent:
'Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/39.0.2171.71 Safari/537.36 Edge/12.0',
version: null
},
{
agent:
'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2486.0 Safari/537.36 Edge/13.10586',
version: null
},
{
agent:
'Mozilla/5.0 (Macintosh; Intel Mac OS X 15_7_3) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/26.0 Safari/605.1.15',
version: null
}
]

uaList.forEach(userAgent => {
let match = userAgent['agent'].match(/MSIE ([^ ;]+)|rv:([^ )]+)/)
let ieVersion = match && (parseFloat(match[1]) || parseFloat(match[2]))
expect(ieVersion).toBe(userAgent['version'])
})
})

it('Should assign the value to the node', function () {
testNode.innerHTML = "<input data-bind='textInput:123' />"
applyBindings(null, testNode)
Expand Down Expand Up @@ -221,21 +250,9 @@ describe('Binding: TextInput', function () {
applyBindings({ someProp: myObservable }, testNode)
expect((testNode.children[0] as HTMLInputElement).value).toEqual('123')
;(testNode.children[0] as HTMLInputElement).value = 'some user-entered value' // setting the value triggers the propertychange event on IE
if (!jasmine.ieVersion || jasmine.ieVersion >= 9) {
triggerEvent(testNode.children[0], 'input')
}
if (jasmine.ieVersion === 9) {
// IE 9 responds to the event asynchronously (see #1788)
waitsFor(
function () {
return myObservable() === 'some user-entered value'
},
'Timeout',
50
)
} else {
expect(myObservable()).toEqual('some user-entered value')
}

triggerEvent(testNode.children[0], 'input')
expect(myObservable()).toEqual('some user-entered value')
})

it('Should update observable on blur event', function () {
Expand Down
88 changes: 42 additions & 46 deletions packages/binding.core/spec/valueBehaviors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -286,53 +286,49 @@ describe('Binding: Value', function () {
expect(myObservable()).toEqual('some value from the server')
})

it('On IE < 10, should handle autofill selection by treating "propertychange" followed by "blur" as a change event', function () {
// This spec describes the awkward choreography of events needed to detect changes to text boxes on IE < 10,
it('Should handle autofill selection by treating "propertychange" followed by "blur" as a change event', function () {
// This spec !no longer! describes the awkward choreography of events needed to detect changes to text boxes on IE < 10,
// because it doesn't fire regular "change" events when the user selects an autofill entry. It isn't applicable
// on IE 10+ or other browsers, because they don't have that problem with autofill.
const isOldIE = jasmine.ieVersion && jasmine.ieVersion < 10

if (isOldIE) {
const myObservable = observable(123).extend({ notify: 'always' })
let numUpdates = 0
myObservable.subscribe(function () {
numUpdates++
})
testNode.innerHTML = "<input data-bind='value:someProp' />"
applyBindings({ someProp: myObservable }, testNode)

// Simulate a blur occurring before the first real property change.
// See that no 'update' event fires.
triggerEvent(testNode.childNodes[0], 'focus')
triggerEvent(testNode.childNodes[0], 'blur')
expect(numUpdates).toEqual(0)

// Simulate:
// 1. Select from autofill
// 2. Modify the textbox further
// 3. Tab out of the textbox
// --- should be treated as a single change
testNode.childNodes[0].value = 'some user-entered value'
triggerEvent(testNode.childNodes[0], 'propertychange')
triggerEvent(testNode.childNodes[0], 'change')
expect(myObservable()).toEqual('some user-entered value')
expect(numUpdates).toEqual(1)
triggerEvent(testNode.childNodes[0], 'blur')
expect(numUpdates).toEqual(1)

// Simulate:
// 1. Select from autofill
// 2. Tab out of the textbox
// 3. Reselect, edit, then tab out of the textbox
// --- should be treated as two changes (one after step 2, one after step 3)
testNode.childNodes[0].value = 'different user-entered value'
triggerEvent(testNode.childNodes[0], 'propertychange')
triggerEvent(testNode.childNodes[0], 'blur')
expect(myObservable()).toEqual('different user-entered value')
expect(numUpdates).toEqual(2)
triggerEvent(testNode.childNodes[0], 'change')
expect(numUpdates).toEqual(3)
}
// on IE 10+ or other browsers, because they don't have that problem with autofill. We have change the test case, so it runs on modern browsers.

const myObservable = observable(123).extend({ notify: 'always' })
let numUpdates = 0
myObservable.subscribe(function () {
numUpdates++
})
testNode.innerHTML = "<input data-bind='value:someProp' />"
applyBindings({ someProp: myObservable }, testNode)

// Simulate a blur occurring before the first real property change.
// See that no 'update' event fires.
triggerEvent(testNode.childNodes[0], 'focus')
triggerEvent(testNode.childNodes[0], 'blur')
expect(numUpdates).toEqual(0)

// Simulate:
// 1. Select from autofill
// 2. Modify the textbox further
// 3. Tab out of the textbox
testNode.childNodes[0].value = 'some user-entered value'
//IE: triggerEvent(testNode.childNodes[0], 'propertychange')
triggerEvent(testNode.childNodes[0], 'change')
expect(myObservable()).toEqual('some user-entered value')
expect(numUpdates).toEqual(1)
triggerEvent(testNode.childNodes[0], 'blur')
expect(numUpdates).toEqual(1)

// Simulate:
// 1. Select from autofill
// 2. Tab out of the textbox
// 3. Reselect, edit, then tab out of the textbox
testNode.childNodes[0].value = 'different user-entered value'
//IE: triggerEvent(testNode.childNodes[0], 'propertychange')
triggerEvent(testNode.childNodes[0], 'blur')
expect(myObservable()).toEqual('some user-entered value')
expect(numUpdates).toEqual(1)
triggerEvent(testNode.childNodes[0], 'change')
expect(numUpdates).toEqual(2)
expect(myObservable()).toEqual('different user-entered value')
})

it('Should bind to file inputs but not allow setting an non-empty value', function () {
Expand Down
6 changes: 2 additions & 4 deletions packages/binding.core/src/attr.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { setElementName, objectForEach } from '@tko/utils'
import { objectForEach } from '@tko/utils'

import { unwrap } from '@tko/observable'

Expand Down Expand Up @@ -34,12 +34,10 @@ export const attr = {
}
}

// Treat "name" specially - although you can think of it as an attribute, it also needs
// special handling on older versions of IE (https://github.com/SteveSanderson/knockout/pull/333)
// Deliberately being case-sensitive here because XHTML would regard "Name" as a different thing
// entirely, and there's no strong reason to allow for such casing in HTML.
if (attrName === 'name') {
setElementName(element, toRemove ? '' : attrValue)
element.name = toRemove ? '' : attrValue
}
})
}
Expand Down
4 changes: 0 additions & 4 deletions packages/binding.core/src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import {
arrayIndexOf,
setOptionNodeSelectionState,
triggerEvent,
ensureSelectElementIsRenderedCorrectly,
selectExtensions
} from '@tko/utils'

Expand Down Expand Up @@ -191,9 +190,6 @@ export const options = {
}
})

// Workaround for IE bug
ensureSelectElementIsRenderedCorrectly(element)

if (previousScrollTop && Math.abs(previousScrollTop - element.scrollTop) > 20) {
element.scrollTop = previousScrollTop
}
Expand Down
122 changes: 19 additions & 103 deletions packages/binding.core/src/textInput.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { ieVersion, safeSetTimeout, options, arrayForEach, domData, registerEventHandler } from '@tko/utils'
import { safeSetTimeout, options, arrayForEach, domData, registerEventHandler } from '@tko/utils'

import { unwrap } from '@tko/observable'

import { BindingHandler } from '@tko/bind'

let operaVersion, safariVersion, firefoxVersion
let operaVersion, safariVersion, firefoxVersion, ieVersion

/**
* TextInput binding handler for modern browsers (legacy below).
Expand Down Expand Up @@ -100,95 +100,11 @@ class TextInput extends BindingHandler {
}
}

/**
* Legacy Input Classes, below
*/
class TextInputIE extends TextInput {
activeElement: any
constructor(...args: [any]) {
super(...args)

let version: number
if (ieVersion instanceof Array) {
version = parseInt(ieVersion[1], 10)
} else {
version = ieVersion ?? 0
}
if (version < 11) {
// Internet Explorer <= 8 doesn't support the 'input' event, but does include 'propertychange' that fires whenever
// any property of an element changes. Unlike 'input', it also fires if a property is changed from JavaScript code,
// but that's an acceptable compromise for this binding. IE 9 and 10 support 'input', but since they don't always
// fire it when using autocomplete, we'll use 'propertychange' for them also.
this.addEventListener('propertychange', event => event.propertyName === 'value' && this.updateModel(event))
}

if (version >= 8 && version < 10) {
//this.watchForSelectionChangeEvent() //FIXME: Cannot work without element => EXCEPTION
this.addEventListener('dragend', 'deferUpdateModel')
}
}

override eventsIndicatingSyncValueChange() {
//keypress: All versions (including 11) of Internet Explorer have a bug that they don't generate an input or propertychange event when ESC is pressed
return [...super.eventsIndicatingSyncValueChange(), 'keypress']
}

// IE 8 and 9 have bugs that prevent the normal events from firing when the value changes.
// But it does fire the 'selectionchange' event on many of those, presumably because the
// cursor is moving and that counts as the selection changing. The 'selectionchange' event is
// fired at the document level only and doesn't directly indicate which element changed. We
// set up just one event handler for the document and use 'activeElement' to determine which
// element was changed.
selectionChangeHandler(event) {
const target = this.activeElement

const handler = target && domData.get(target, 'selectionChangeHandlerName')
if (handler) {
handler(event)
}
}

// All variables are not found!
watchForSelectionChangeEvent(element?) {
if (!element) throw new Error('Broken IE-Support: 8 to 9')

const ownerDoc = element.ownerDocument

if (!domData.get(ownerDoc, 'selectionChangeRegisteredName')) {
domData.set(ownerDoc, 'selectionChangeRegisteredName', true)
registerEventHandler(ownerDoc, 'selectionchange', this.selectionChangeHandler.bind(ownerDoc))
}
//domData.set(ownerDoc, 'selectionChangeRegisteredName', handler) //FIXME: handler not defined
}
}

///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

// IE 8 and 9 have bugs that prevent the normal events from firing when the value changes.
// But it does fire the 'selectionchange' event on many of those, presumably because the
// cursor is moving and that counts as the selection changing. The 'selectionchange' event is
// fired at the document level only and doesn't directly indicate which element changed. We
// set up just one event handler for the document and use 'activeElement' to determine which
// element was changed.
class TextInputIE9 extends TextInputIE {
override updateModel(...args: [any]) {
// IE9 will mess up the DOM if you handle events synchronously which results in DOM changes (such as other bindings);
// so we'll make sure all updates are asynchronous
this.deferUpdateModel(...args)
}
}

class TextInputIE8 extends TextInputIE {
eventsIndicatingValueChange() {
// IE 8 has a bug where it fails to fire 'propertychange' on the first update following a value change from
// JavaScript code. It also doesn't fire if you clear the entire value. To fix this, we bind to the following
// events too.
// keypress: All versions (including 11) of Internet Explorer have a bug that they don't generate an input or propertychange event when ESC is pressed
// keyup: A single keystoke
// keydown: First character when a key is held down

return [...super.eventsIndicatingSyncValueChange(), 'keyup', 'keydown']
}
}

// Safari <5 doesn't fire the 'input' event for <textarea> elements (it does fire 'textInput'
Expand Down Expand Up @@ -225,23 +141,23 @@ if (w.navigator) {
const parseVersion = matches => matches && parseFloat(matches[1])
const userAgent = w.navigator.userAgent
const isChrome = userAgent.match(/Chrome\/([^ ]+)/)
// Detect various browser versions because some old versions don't fully support the 'input' event
operaVersion = w.opera && w.opera.version && parseInt(w.opera.version())
safariVersion = parseVersion(userAgent.match(/Version\/([^ ]+) Safari/))
firefoxVersion = parseVersion(userAgent.match(/Firefox\/([^ ]*)/))
if (!isChrome) {
// Detect various browser versions because some old versions don't fully support the 'input' event
operaVersion = w.opera && w.opera.version && parseInt(w.opera.version())
safariVersion = parseVersion(userAgent.match(/Version\/([^ ]+) Safari/))
firefoxVersion = parseVersion(userAgent.match(/Firefox\/([^ ]*)/))
const ieMatch = userAgent.match(/MSIE ([^ ;]+)|rv:([^ )]+)/)
ieVersion = ieMatch && (parseFloat(ieMatch[1]) || parseFloat(ieMatch[2]))
}
}

export const textInput =
ieVersion === 8
? TextInputIE8
: ieVersion === 9
? TextInputIE9
: ieVersion
? TextInputIE
: safariVersion && safariVersion < 5
? TextInputLegacySafari
: operaVersion < 11
? TextInputLegacyOpera
: firefoxVersion && firefoxVersion < 4
? TextInputLegacyFirefox
: TextInput
ieVersion && ieVersion <= 11
? TextInputIE
: safariVersion && safariVersion < 5
? TextInputLegacySafari
: operaVersion && operaVersion < 11
? TextInputLegacyOpera
: firefoxVersion && firefoxVersion < 4
? TextInputLegacyFirefox
: TextInput
Comment on lines +157 to +165
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 10, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: IE9 inadvertently included in version check.

The condition ieVersion && ieVersion <= 11 will evaluate to true for IE9, IE10, and IE11, causing TextInputIE to be used for IE9. This contradicts the PR objective to remove IE9 support and the learning that states "IE10/11 support is intentionally retained even though IE9 support has been removed."

🐛 Proposed fix to exclude IE9
 export const textInput =
-  ieVersion && ieVersion <= 11
+  ieVersion && ieVersion >= 10 && ieVersion <= 11
     ? TextInputIE
     : safariVersion && safariVersion < 5
       ? TextInputLegacySafari

Based on learnings, IE9 support was intentionally removed while IE10/11 support is retained.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ieVersion && ieVersion <= 11
? TextInputIE
: safariVersion && safariVersion < 5
? TextInputLegacySafari
: operaVersion && operaVersion < 11
? TextInputLegacyOpera
: firefoxVersion && firefoxVersion < 4
? TextInputLegacyFirefox
: TextInput
ieVersion && ieVersion >= 10 && ieVersion <= 11
? TextInputIE
: safariVersion && safariVersion < 5
? TextInputLegacySafari
: operaVersion && operaVersion < 11
? TextInputLegacyOpera
: firefoxVersion && firefoxVersion < 4
? TextInputLegacyFirefox
: TextInput
🤖 Prompt for AI Agents
In @packages/binding.core/src/textInput.ts around lines 157 - 165, The IE branch
condition currently uses "ieVersion && ieVersion <= 11" which matches IE9 and
thus enables TextInputIE for IE9; update the condition in the selection
expression that decides TextInputIE so IE9 is explicitly excluded (e.g., require
ieVersion >= 10 && ieVersion <= 11 or ieVersion !== 9 && ieVersion <= 11) so
that TextInputIE is only used for IE10/IE11 and IE9 falls through to the non-IE
legacy path.

Copy link
Member Author

Choose a reason for hiding this comment

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

However, it is correct that this condition also applied to smaller versions of IE. The official support for IE9 and explicit IE9 switches are removed with this pr. Such partial support may remain.

Copy link
Contributor

Choose a reason for hiding this comment

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

@phillipc, thank you for the clarification! I understand now — you've removed the explicit IE9-specific handlers and switches (like TextInputIE9), but you're allowing IE9 to fall through to the IE10/11 handler if it happens to work. This is a pragmatic approach where explicit IE9 support is dropped without actively blocking it.

I appreciate you explaining the distinction between removing official support versus allowing partial compatibility. This makes sense for a gradual deprecation strategy.


✏️ Learnings added
Learnt from: phillipc
Repo: knockout/tko PR: 233
File: packages/binding.core/src/textInput.ts:157-165
Timestamp: 2026-01-12T18:24:54.434Z
Learning: In the TKO codebase, the IE9 removal strategy removes explicit IE9 switches and IE9-specific code paths, but allows partial support to remain. For example, in packages/binding.core/src/textInput.ts, the condition `ieVersion && ieVersion <= 11` intentionally allows IE9 to use the same handler as IE10/11, even though explicit IE9 support has been removed from the project.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: phillipc
Repo: knockout/tko PR: 233
File: packages/binding.core/src/value.ts:64-72
Timestamp: 2026-01-03T18:06:10.239Z
Learning: In packages/binding.core/src/value.ts, the ieAutoCompleteHackNeeded getter and related propertyChange/focus/blur event handlers for IE10/11 autocomplete workarounds should be kept, as IE10/11 support is intentionally retained even though IE9 support has been removed.

Loading