Open
Bug 596100
Opened 13 years ago
Updated 1 year ago
HTMLInputElement should be changing the editor state instead of nsTextControlFrame
Categories
(Core :: DOM: Core & HTML, defect, P5)
Core
DOM: Core & HTML
Tracking
()
NEW
People
(Reporter: mounir, Unassigned)
References
Details
The editor is now owned by the input element but the frame is still changing some modes linked to attributes. I have to change that for bug 557087 so I might change for all kind of editor states linked to content attributes. Ehsan, let me know if that doesn't seem as easy as it looks.
Comment 1•13 years ago
|
||
I'm not really sure what you're trying to do here. Can you be more specific, please?
Reporter | ||
Comment 2•13 years ago
|
||
nsTextControlFrame is changing the states of the editor when an attribute is changed (in nsTextControlFrame::AttributeChanged). For example, this is called when the disabled state is set: PRUint32 flags; editor->GetFlags(&flags); if (AttributeExists(nsGkAtoms::disabled)) { // set disabled flags |= nsIPlaintextEditor::eEditorDisabledMask; selCon->SetDisplaySelection(nsISelectionController::SELECTION_OFF); if (nsContentUtils::IsFocusedContent(mContent)) selCon->SetCaretEnabled(PR_FALSE); } It seems better to have this done by the content given that the content now owns the editor and is more appropriate for attribute changes actions.
Reporter | ||
Comment 3•13 years ago
|
||
BTW, I've open this bug because text controls can know be disabled like this: <fieldset disabled> <input> </fieldset> In that case, the editor inside the input will be enabled. Hopefully, there are no real consequences given that the user can't interact with the element.
Comment 4•13 years ago
|
||
(In reply to comment #2) > nsTextControlFrame is changing the states of the editor when an attribute is > changed (in nsTextControlFrame::AttributeChanged). > For example, this is called when the disabled state is set: > PRUint32 flags; > editor->GetFlags(&flags); > if (AttributeExists(nsGkAtoms::disabled)) > { // set disabled > flags |= nsIPlaintextEditor::eEditorDisabledMask; > selCon->SetDisplaySelection(nsISelectionController::SELECTION_OFF); > if (nsContentUtils::IsFocusedContent(mContent)) > selCon->SetCaretEnabled(PR_FALSE); > } > > It seems better to have this done by the content given that the content now > owns the editor and is more appropriate for attribute changes actions. Ah, OK. Yes, moving those pieces of code to content should be safe, as long as you handle the case where the operation requires a frame to be bound to the content node, and the content node does not have a frame.
Comment 5•13 years ago
|
||
(In reply to comment #3) > BTW, I've open this bug because text controls can know be disabled like this: > <fieldset disabled> > <input> > </fieldset> > > In that case, the editor inside the input will be enabled. Hopefully, there are > no real consequences given that the user can't interact with the element. What types of consequences are you worried about here?
Reporter | ||
Comment 6•13 years ago
|
||
(In reply to comment #5) > (In reply to comment #3) > > BTW, I've open this bug because text controls can know be disabled like this: > > <fieldset disabled> > > <input> > > </fieldset> > > > > In that case, the editor inside the input will be enabled. Hopefully, there are > > no real consequences given that the user can't interact with the element. > > What types of consequences are you worried about here? None actually now. I was worried about that before (when I saw the frame was enabling/disabling the editor depending of the disabled attribute) but an enabled editor inside a disabled text field doesn't seem to have any consequence. It would be better to move that to the content though.
Comment 7•13 years ago
|
||
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #3) > > > BTW, I've open this bug because text controls can know be disabled like this: > > > <fieldset disabled> > > > <input> > > > </fieldset> > > > > > > In that case, the editor inside the input will be enabled. Hopefully, there are > > > no real consequences given that the user can't interact with the element. > > > > What types of consequences are you worried about here? > > None actually now. I was worried about that before (when I saw the frame was > enabling/disabling the editor depending of the disabled attribute) but an > enabled editor inside a disabled text field doesn't seem to have any > consequence. > It would be better to move that to the content though. Wait, that shouldn't happen. Setting the content's disabled attribute should set the disabled flag on the editor, doesn't it?
Reporter | ||
Comment 8•13 years ago
|
||
(In reply to comment #7) > Wait, that shouldn't happen. Setting the content's disabled attribute should > set the disabled flag on the editor, doesn't it? As I said in comment 3, the element can now be disabled because of its fieldset so the disabled attribute might not be set.
Comment 9•13 years ago
|
||
(In reply to comment #8) > (In reply to comment #7) > > Wait, that shouldn't happen. Setting the content's disabled attribute should > > set the disabled flag on the editor, doesn't it? > > As I said in comment 3, the element can now be disabled because of its fieldset > so the disabled attribute might not be set. I see. Yes. That's the sort of problem that we can solve if we move the handling of the disabled attribute to the content node. I think relying on the fact that things work _now_ is fragile.
Reporter | ||
Comment 10•13 years ago
|
||
(In reply to comment #9) > I see. Yes. That's the sort of problem that we can solve if we move the > handling of the disabled attribute to the content node. I think relying on the > fact that things work _now_ is fragile. Do you think it worths being fixed for 2.0? (to know if I move that to my TODO list)
Comment 11•13 years ago
|
||
(In reply to comment #10) > (In reply to comment #9) > > I see. Yes. That's the sort of problem that we can solve if we move the > > handling of the disabled attribute to the content node. I think relying on the > > fact that things work _now_ is fragile. > > Do you think it worths being fixed for 2.0? (to know if I move that to my TODO > list) Hmm, not really, unless there's a web compat bug or a regression which would be fixed by this. This is kind of risky business. I fixed the last regression from moving editor to content just last week. ;-)
Reporter | ||
Updated•12 years ago
|
Summary: HTMLInputElement should change the editor state, not the frame → HTMLInputElement should be changing the editor state instead of nsTextControlFrame
Comment 12•12 years ago
|
||
(In reply to Ehsan Akhgari [:ehsan] from comment #11) > (In reply to comment #10) > > (In reply to comment #9) > > > I see. Yes. That's the sort of problem that we can solve if we move the > > > handling of the disabled attribute to the content node. I think relying on the > > > fact that things work _now_ is fragile. > > > > Do you think it worths being fixed for 2.0? (to know if I move that to my TODO > > list) > > Hmm, not really, unless there's a web compat bug or a regression which would > be fixed by this. This is kind of risky business. I fixed the last > regression from moving editor to content just last week. ;-) Make that yesterday! ;)
Comment 13•5 years ago
|
||
https://bugzilla.mozilla.org/show_bug.cgi?id=1472046 Move all DOM bugs that haven’t been updated in more than 3 years and has no one currently assigned to P5. If you have questions, please contact :mdaly.
Priority: -- → P5
Updated•1 year ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•