Summary: | Bugs in textarea.selectionStart/selectionEnd (as seen on wikipedia) | ||
---|---|---|---|
Product: | [Applications] konqueror | Reporter: | Marijn Schouten <hkBst> |
Component: | khtml ecma | Assignee: | Konqueror Developers <konq-bugs> |
Status: | RESOLVED WORKSFORME | ||
Severity: | normal | CC: | maksim |
Priority: | NOR | ||
Version: | unspecified | ||
Target Milestone: | --- | ||
Platform: | Gentoo Packages | ||
OS: | Linux | ||
Latest Commit: | Version Fixed In: | ||
Sentry Crash Report: |
Description
Marijn Schouten
2006-02-03 17:04:58 UTC
Konqueror doesn't show them because Wikipedia disables them for Konqueror: // Don't generate buttons for browsers which don't fully // support it. if (!document.selection && !is_gecko) { return false; } If you spoof and force them to show, they don't work. >If you spoof and force them to show, they don't work.
Fact remains that it doesn't work. So I'd hardly call this bug resolved.
see previous comment That's because you're using the obsolete ;-) 3.5.1, and not the development version, which supports the Mozilla-proprietary feature the site uses (though I still have to fix a few bugs in it) Retitling. If it's committed to trunk, close it. Wrong title. document.selection is an IEism here, and not relevant. SVN commit 507283 by orlovich: Make selectionStart/selectionEnd more reliable -- in particular WikiPedia insertion things seem to work very well right now. The bulk of the fix is in moving CR LF expansion to form submission, so the renderer and the DOM don't fight about whether to use CR or CR LF. There is still one case that's broken: when physical wrap text areas are first created, they wrap the text. but the DOM doesn't know that. Not sure how to fix that. Also, remove the onChange call in ::value -- too dangerous -- and re-fix onChange differently, by keeping track of need to emit it separately from the m_value dirtiness. CCBUG:121297 M +41 -17 html/html_formimpl.cpp M +2 -1 html/html_formimpl.h M +19 -52 rendering/render_form.cpp --- branches/KDE/3.5/kdelibs/khtml/html/html_formimpl.cpp #507282:507283 @@ -2550,7 +2550,8 @@ m_rows = 2; m_cols = 20; m_wrap = ta_Virtual; - m_dirtyvalue = true; + m_changed = false; + m_dirtyvalue = true; m_initialized = false; m_unsubmittedFormChange = false; } @@ -2649,12 +2650,47 @@ _style->deref(); } + +static QString expandLF(const QString& s) +{ + // LF -> CRLF + unsigned crs = s.contains( '\n' ); + if (crs == 0) + return s; + unsigned len = s.length(); + + QString r; + r.reserve(len + crs + 1); + unsigned pos2 = 0; + for(unsigned pos = 0; pos < len; pos++) + { + QChar c = s.at(pos); + switch(c.unicode()) + { + case '\n': + r[pos2++] = '\r'; + r[pos2++] = '\n'; + break; + + case '\r': + break; + + default: + r[pos2++]= c; + break; + } + } + r.squeeze(); + return r; +} + + bool HTMLTextAreaElementImpl::encoding(const QTextCodec* codec, encodingList& encoding, bool) { if (name().isEmpty()) return false; encoding += fixUpfromUnicode(codec, name().string()); - encoding += fixUpfromUnicode(codec, value().string()); + encoding += fixUpfromUnicode(codec, expandLF(value().string())); return true; } @@ -2664,14 +2700,14 @@ setValue(defaultValue()); } + DOMString HTMLTextAreaElementImpl::value() { if ( m_dirtyvalue) { if ( m_render && m_initialized ) { RenderTextArea* renderArea = static_cast<RenderTextArea*>( m_render ); m_value = renderArea->text(); - m_dirtyvalue = false; // before onChange (#100963) - onChange(); + m_dirtyvalue = false; } else { m_value = defaultValue().string(); m_initialized = true; @@ -2787,19 +2823,7 @@ long HTMLTextAreaElementImpl::textLength() { - //First, get the value. This is like ::value, only pure. - DOMString val = m_value; - if (m_dirtyvalue) { - if ( m_render && m_initialized ) { - RenderTextArea* renderArea = static_cast<RenderTextArea*>( m_render ); - val = renderArea->text(); - } else { - val = defaultValue(); - } - } - - //now we can get the length. - return val.length(); + return value().length(); } // ------------------------------------------------------------------------- --- branches/KDE/3.5/kdelibs/khtml/html/html_formimpl.h #507282:507283 @@ -566,7 +566,8 @@ int m_cols; WrapMethod m_wrap; QString m_value; - bool m_dirtyvalue: 1; + bool m_changed: 1; //States whether the contents has been editted + bool m_dirtyvalue: 1; //States whether m_value is out-of-date compared to the renderer or default bool m_unsubmittedFormChange: 1; bool m_initialized: 1; }; --- branches/KDE/3.5/kdelibs/khtml/rendering/render_form.cpp #507282:507283 @@ -1566,6 +1566,10 @@ if ( w && element()->m_dirtyvalue ) { element()->m_value = text(); element()->m_dirtyvalue = false; + } + + if ( w && element()->m_changed ) { + element()->m_changed = false; element()->onChange(); } } @@ -1649,39 +1653,7 @@ RenderFormElement::close(); } -static QString expandLF(const QString& s) -{ - // LF -> CRLF - unsigned crs = s.contains( '\n' ); - if (crs == 0) - return s; - unsigned len = s.length(); - QString r; - r.reserve(len + crs + 1); - unsigned pos2 = 0; - for(unsigned pos = 0; pos < len; pos++) - { - QChar c = s.at(pos); - switch(c.unicode()) - { - case '\n': - r[pos2++] = '\r'; - r[pos2++] = '\n'; - break; - - case '\r': - break; - - default: - r[pos2++]= c; - break; - } - } - r.squeeze(); - return r; -} - QString RenderTextArea::text() { QString txt; @@ -1709,19 +1681,9 @@ else txt = w->text(); - return expandLF(txt); + return txt; } -static int expandedCnt(unsigned short code) -{ - if (code == '\n') - return 2; - else if (code == '\r') - return 0; - else - return 1; -} - int RenderTextArea::queryParagraphInfo(int para, Mode m, int param) { /* We have to be a bit careful here, as we need to match up the positions to what our value returns here*/ @@ -1732,6 +1694,7 @@ QString paragraphText = w->text(para); int pl = w->paragraphLength(para); + int physicalPL = pl; if (m == ParaPortionLength) pl = param; @@ -1739,25 +1702,28 @@ //Go through all the chars of paragraph, and count line changes, chars, etc. int lindex = w->lineOfChar(para, 0); for (int c = 0; c < pl; ++c) { - if (lindex != w->lineOfChar(para, c)) { - length += 2; - lindex = w->lineOfChar(para, c); + ++length; + // Is there a change after this char? + if (c+1 < physicalPL && lindex != w->lineOfChar(para, c+1)) { + lindex = w->lineOfChar(para, c+1); + ++length; } - length += expandedCnt(paragraphText.at(c).unicode()); if (m == ParaPortionOffset && length > param) return c; } } else { - //Make sure to count the LF, CR as appropriate.. + //Make sure to count the LF, CR as appropriate. ### this is stupid now, simplify for (int c = 0; c < pl; ++c) { - length += expandedCnt(paragraphText.at(c).unicode()); + ++length; if (m == ParaPortionOffset && length > param) return c; } } if (m == ParaPortionOffset) return pl; - return length; + if (m == ParaPortionLength) + return length; + return length + 1; } long RenderTextArea::computeCharOffset(int para, int index) { @@ -1766,7 +1732,7 @@ long pos = 0; for (int cp = 0; cp < para; ++cp) - pos += queryParagraphInfo(cp, ParaLength) + 2; + pos += queryParagraphInfo(cp, ParaLength); if (index >= 0) pos += queryParagraphInfo(para, ParaPortionLength, index); @@ -1787,7 +1753,7 @@ long endPos = 0; long startPos = 0; for (int p = 0; p < w->paragraphs(); ++p) { - int len = queryParagraphInfo(p, ParaLength) + 2; + int len = queryParagraphInfo(p, ParaLength); endPos += len; if (endPos > offset) { containingPar = p; @@ -1815,6 +1781,7 @@ void RenderTextArea::slotTextChanged() { element()->m_dirtyvalue = true; + element()->m_changed = true; if (element()->m_value != text()) element()->m_unsubmittedFormChange = true; } If I spoof as firefox 1.0 with konqueror 3.5.2 all insertion buttons work. Great job Maksim! I'm now trying to get wikipedia to recognize this at http://bugzilla.wikimedia.org/show_bug.cgi?id=5572 they seem to want a way to check for the required features. Just checking for selectionStart should work --- and I -think- that's what they did. Ok, this issue has been completely resolved. |